fix(hipdnn): keep left_bound under causal_mask in SDPA engines

dnn-providers/hip-kernel-provider/src/engines/asm_sdpa_engine/plans/SdpaBwdPlanBuilder.cpp

@@ -959,7 +959,8 @@ void SdpaBwdPlanBuilder::buildPlan(
             "(isApplicable should have rejected)");
     }
     const auto& dataTypeId = *dataTypeIdOpt;
-    auto maskType = plan_utils::getMaskType(sdpaAttrs);
+    const auto resolvedMask = plan_utils::resolveMask(sdpaAttrs);
+    const auto maskType = resolvedMask.type;
     auto batchMode = getBatchMode(sdpaAttrs);
     const int bf16CvtValue = (dataTypeId == "fp16") ? BF16_CVT_FP16_SENTINEL
                                                     : static_cast<int>(getRoundingMode(sdpaAttrs));
@@ -1173,15 +1174,12 @@ void SdpaBwdPlanBuilder::buildPlan(
     params.maskOrdinal = static_cast<int32_t>(maskType);
     if(maskType == MaskType::SLIDING_WINDOW)
     {
-        params.windowLeft = sdpaAttrs.left_bound().has_value()
-                                ? static_cast<int32_t>(sdpaAttrs.left_bound().value())
-                                : -1;
-        params.windowRight = sdpaAttrs.right_bound().has_value()
-                                 ? static_cast<int32_t>(sdpaAttrs.right_bound().value())
-                                 : -1;
-        params.topLeftAlignment
-            = sdpaAttrs.diagonal_alignment()
-              != hipdnn_flatbuffers_sdk::data_objects::DiagonalAlignment::BOTTOM_RIGHT;
+        // Resolved bounds, not the raw attributes: a deprecated causal boolean
+        // plus left_bound is a window whose right bound and alignment come from
+        // the boolean, not from right_bound / diagonal_alignment.
+        params.windowLeft = static_cast<int32_t>(resolvedMask.left);
+        params.windowRight = static_cast<int32_t>(resolvedMask.right);
+        params.topLeftAlignment = resolvedMask.topLeft;
     }
 
     if(postKernel)

dnn-providers/hip-kernel-provider/src/engines/asm_sdpa_engine/plans/SdpaPlanUtils.hpp

@@ -55,17 +55,30 @@ enum class MaskType : int
     SLIDING_WINDOW = 3
 };
 
-// Classify the mask requested by an SDPA (forward or backward) attribute set.
+// The mask an SDPA (forward or backward) attribute set asks for: its kind plus
+// the band it describes in the left_bound / right_bound / alignment convention
+// (-1 = unbounded). `left`, `right` and `topLeft` are what a SLIDING_WINDOW
+// kernel needs; for the other kinds they restate the mask.
+struct ResolvedMask
+{
+    MaskType type = MaskType::NO_MASK;
+    int64_t left = -1;
+    int64_t right = -1;
+    bool topLeft = true;
+};
+
+// Resolve the mask requested by an SDPA (forward or backward) attribute set.
 //
 // Two sources can describe the mask: the modern left_bound / right_bound /
 // diagonal_alignment trio, and the deprecated causal_mask /
-// causal_mask_bottom_right booleans. When a deprecated boolean is set it takes
-// precedence and the modern trio is ignored; otherwise the trio is
-// authoritative. The two deprecated booleans are mutually exclusive, so setting
-// both throws HipdnnPluginException(INVALID_VALUE).
-//
-// Guaranteeing the two parameter sets agree belongs in the hipDNN frontend; this
-// helper only resolves which source wins for dispatch.
+// causal_mask_bottom_right booleans. A deprecated boolean fixes the diagonal
+// (right bound 0) and its alignment, overriding right_bound and
+// diagonal_alignment, but it keeps a real left_bound: causal_mask plus
+// left_bound is a causal sliding window, the way cuDNN's set_causal_mask(true)
+// plus set_sliding_window_length(n) spells one. This matches the CPU and GPU
+// SDPA references (extractDiagonalBandParams). Without a deprecated boolean
+// the trio is authoritative. The two deprecated booleans are mutually
+// exclusive, so setting both throws HipdnnPluginException(INVALID_VALUE).
 //
 // Absence-awareness: the generated flatbuffer accessors expose the causal_mask*
 // fields as plain bool defaulting to false, with no has_*() accessor.
@@ -76,7 +89,7 @@ enum class MaskType : int
 // specified trio (e.g. only right_bound = 0) still derives a mask rather than
 // silently falling back to NO_MASK.
 template <typename SdpaAttrsT>
-MaskType getMaskType(const SdpaAttrsT& attrs)
+ResolvedMask resolveMask(const SdpaAttrsT& attrs)
 {
     using namespace hipdnn_flatbuffers_sdk::data_objects;
 
@@ -92,33 +105,39 @@ MaskType getMaskType(const SdpaAttrsT& attrs)
             "but both are set");
     }
 
-    // Deprecated booleans take precedence: when either is set, defer to it and
-    // ignore the modern bounds trio.
-    if(causalDeprecated)
+    ResolvedMask mask;
+    mask.left = attrs.left_bound().has_value() ? attrs.left_bound().value() : -1;
+    if(causalDeprecated || bottomRightDeprecated)
     {
-        return MaskType::TOP_LEFT_CAUSAL;
+        mask.right = 0;
+        mask.topLeft = causalDeprecated;
     }
-    if(bottomRightDeprecated)
+    else
     {
-        return MaskType::BOTTOM_RIGHT_CAUSAL;
+        mask.right = attrs.right_bound().has_value() ? attrs.right_bound().value() : -1;
+        mask.topLeft = attrs.diagonal_alignment() != DiagonalAlignment::BOTTOM_RIGHT;
     }
 
-    // No deprecated boolean set: the modern bounds trio is authoritative. An
-    // unset bound means unbounded, represented here as -1, so a partially
-    // specified trio still resolves to the mask it describes.
-    const int64_t left = attrs.left_bound().has_value() ? attrs.left_bound().value() : -1;
-    const int64_t right = attrs.right_bound().has_value() ? attrs.right_bound().value() : -1;
-    if(left == -1 && right == -1) // both unbounded
+    if(mask.left == -1 && mask.right == -1) // both unbounded
+    {
+        mask.type = MaskType::NO_MASK;
+    }
+    else if(mask.left == -1 && mask.right == 0) // causal: attend up to the diagonal
     {
-        return MaskType::NO_MASK;
+        mask.type = mask.topLeft ? MaskType::TOP_LEFT_CAUSAL : MaskType::BOTTOM_RIGHT_CAUSAL;
     }
-    if(left == -1 && right == 0) // causal: attend up to the diagonal
+    else
     {
-        return attrs.diagonal_alignment() == DiagonalAlignment::BOTTOM_RIGHT
-                   ? MaskType::BOTTOM_RIGHT_CAUSAL
-                   : MaskType::TOP_LEFT_CAUSAL;
+        mask.type = MaskType::SLIDING_WINDOW; // anything else is a sliding window
     }
-    return MaskType::SLIDING_WINDOW; // anything else is a sliding window
+    return mask;
+}
+
+// The kind of mask resolveMask() derives; see there for the precedence rules.
+template <typename SdpaAttrsT>
+MaskType getMaskType(const SdpaAttrsT& attrs)
+{
+    return resolveMask(attrs).type;
 }
 
 // =============================================================================

dnn-providers/hip-kernel-provider/src/engines/hip_flash2_engine/HipFlash2FwdPlanBuilder.cpp

@@ -355,7 +355,7 @@ Flash2FwdParams HipFlash2FwdPlanBuilder::extractParams(const Handle& /*handle*/,
         p.attnScale = attrs.attn_scale_value().value();
     }
 
-    p.causal = attrs.causal_mask();
+    p.causal = requestsCausal(attrs);
 
     p.qStrideBatch = q->strides()->Get(0);
     p.qStrideHead = q->strides()->Get(1);
@@ -373,4 +373,12 @@ Flash2FwdParams HipFlash2FwdPlanBuilder::extractParams(const Handle& /*handle*/,
     return p;
 }
 
+bool HipFlash2FwdPlanBuilder::requestsCausal(const data_objects::SdpaAttributes& attrs)
+{
+    // The same classification isApplicable gates on, so a causal mask spelled with
+    // the bounds trio is masked too.
+    return asm_sdpa_engine::plan_utils::getMaskType(attrs)
+           == asm_sdpa_engine::plan_utils::MaskType::TOP_LEFT_CAUSAL;
+}
+
 } // namespace hip_flash2_engine

dnn-providers/hip-kernel-provider/src/engines/hip_flash2_engine/HipFlash2FwdPlanBuilder_v2.hpp

@@ -12,6 +12,7 @@
 #pragma once
 
 #include <hipdnn_flatbuffers_sdk/data_objects/knob_value_generated.h>
+#include <hipdnn_flatbuffers_sdk/data_objects/sdpa_attributes_generated.h>
 
 #include <hipdnn_plugin_sdk/interfaces/IPlanBuilder.hpp>
 #include <vector>
@@ -69,6 +70,11 @@ class HipFlash2FwdPlanBuilder : public hipdnn_plugin_sdk::IPlanBuilder<Handle, S
         const Handle& handle,
         const hipdnn_flatbuffers_sdk::flatbuffer_utilities::IGraph& opGraph) const override;
 
+    /// Whether the kernel's causal flag must be set for this SDPA node: true for a
+    /// top-left causal mask however it is spelled (causal_mask, or right_bound 0
+    /// with top-left alignment). isApplicable has already declined every other mask.
+    static bool requestsCausal(const hipdnn_flatbuffers_sdk::data_objects::SdpaAttributes& attrs);
+
 private:
     /// Extract Flash2FwdParams from a validated SDPA graph.
     static Flash2FwdParams

dnn-providers/hip-kernel-provider/src/tests/engines/asm_sdpa_engine/TestSdpaBwdPlanBuilder.cpp

@@ -883,13 +883,14 @@ TEST_F(TestSdpaBwdPlanBuilder, IsApplicableRejectsAsymmetricHdim)
 }
 
 // =============================================================================
-// Canonical mask-attribute policy (plan_utils::getMaskType)
+// Canonical mask-attribute policy (plan_utils::resolveMask / getMaskType)
 // =============================================================================
 //
 // These tests exercise the shared mask-precedence policy directly through
-// plan_utils::getMaskType rather than through isApplicable. When a deprecated
-// causal boolean is set it takes precedence over the modern bounds trio; only
-// setting both deprecated booleans at once throws. The policy is
+// plan_utils::resolveMask rather than through isApplicable. A deprecated causal
+// boolean fixes the diagonal and its alignment but keeps a real left_bound
+// (a causal sliding window); only setting both deprecated booleans at once
+// throws. The policy is
 // hardware-agnostic (it runs before any device dispatch and independent of the
 // kernel registry), so the assertions are meaningful on any device — including
 // this gfx950 box. The backward isApplicable cannot be used here: it rejects
@@ -997,16 +998,22 @@ flatbuffers::FlatBufferBuilder createSdpaBwdGraphWithMask(
     return builder;
 }
 
-// Resolve the SDPA backward attributes from a graph buffer and classify the mask.
-plan_utils::MaskType classifyMask(const flatbuffers::FlatBufferBuilder& builder)
+// Resolve the SDPA backward attributes from a graph buffer and resolve the mask.
+plan_utils::ResolvedMask resolveMaskOf(const flatbuffers::FlatBufferBuilder& builder)
 {
     const hipdnn_flatbuffers_sdk::flatbuffer_utilities::GraphWrapper graphWrapper(
         builder.GetBufferPointer(), builder.GetSize());
     const auto& attrs
         = graphWrapper.nodeWrappers()
               .front()
               ->attributesAs<hipdnn_flatbuffers_sdk::data_objects::SdpaBackwardAttributes>();
-    return plan_utils::getMaskType(attrs);
+    return plan_utils::resolveMask(attrs);
+}
+
+// Resolve the SDPA backward attributes from a graph buffer and classify the mask.
+plan_utils::MaskType classifyMask(const flatbuffers::FlatBufferBuilder& builder)
+{
+    return resolveMaskOf(builder).type;
 }
 
 TEST_F(TestSdpaBwdPlanBuilder, IsApplicableRejectsCausalMaskAndBottomRightSetTogether)
@@ -1024,22 +1031,28 @@ TEST_F(TestSdpaBwdPlanBuilder, IsApplicableRejectsCausalMaskAndBottomRightSetTog
     EXPECT_THROW(classifyMask(builder), hipdnn_plugin_sdk::HipdnnPluginException);
 }
 
-TEST_F(TestSdpaBwdPlanBuilder, IsApplicablePrefersCausalMaskOverWindowBounds)
+TEST_F(TestSdpaBwdPlanBuilder, CausalMaskWithLeftBoundIsTopLeftSlidingWindow)
 {
     using namespace hipdnn_flatbuffers_sdk::data_objects;
 
-    // causal_mask=true takes precedence over the bounds trio, even though the
-    // bounds describe a sliding window (left=64, right=64): result is causal.
+    // causal_mask=true plus left_bound is a causal sliding window (cuDNN's
+    // set_causal_mask(true).set_sliding_window_length(n)). The boolean fixes the
+    // diagonal and its alignment, overriding right_bound=64 and BOTTOM_RIGHT, and
+    // keeps left_bound. Serving it as plain causal would widen the window to the
+    // full triangle with no error.
     auto builder = createSdpaBwdGraphWithMask(
         /*causalMask=*/true,
         /*causalMaskBottomRight=*/false,
         flatbuffers::Optional<int64_t>(64),
         flatbuffers::Optional<int64_t>(64),
-        DiagonalAlignment::TOP_LEFT);
+        DiagonalAlignment::BOTTOM_RIGHT);
 
-    plan_utils::MaskType maskType = plan_utils::MaskType::NO_MASK;
-    EXPECT_NO_THROW(maskType = classifyMask(builder));
-    EXPECT_EQ(maskType, plan_utils::MaskType::TOP_LEFT_CAUSAL);
+    plan_utils::ResolvedMask mask;
+    EXPECT_NO_THROW(mask = resolveMaskOf(builder));
+    EXPECT_EQ(mask.type, plan_utils::MaskType::SLIDING_WINDOW);
+    EXPECT_EQ(mask.left, 64);
+    EXPECT_EQ(mask.right, 0);
+    EXPECT_TRUE(mask.topLeft);
 }
 
 TEST_F(TestSdpaBwdPlanBuilder, IsApplicablePrefersBottomRightCausalOverTopLeftBounds)
@@ -1079,23 +1092,25 @@ TEST_F(TestSdpaBwdPlanBuilder, IsApplicableAcceptsConsistentCausalMaskAndBounds)
     EXPECT_EQ(maskType, plan_utils::MaskType::TOP_LEFT_CAUSAL);
 }
 
-TEST_F(TestSdpaBwdPlanBuilder, IsApplicablePrefersBottomRightCausalOverWindowBounds)
+TEST_F(TestSdpaBwdPlanBuilder, BottomRightCausalMaskWithLeftBoundIsBottomRightSlidingWindow)
 {
     using namespace hipdnn_flatbuffers_sdk::data_objects;
 
-    // causal_mask_bottom_right=true takes precedence over the bounds trio, even
-    // though a symmetric sliding window (left=64, right=64) would derive
-    // SLIDING_WINDOW: result is bottom-right causal.
+    // Same rule for causal_mask_bottom_right: a bottom-right causal window of 64,
+    // whatever right_bound and diagonal_alignment say.
     auto builder = createSdpaBwdGraphWithMask(
         /*causalMask=*/false,
         /*causalMaskBottomRight=*/true,
         flatbuffers::Optional<int64_t>(64),
         flatbuffers::Optional<int64_t>(64),
-        DiagonalAlignment::BOTTOM_RIGHT);
+        DiagonalAlignment::TOP_LEFT);
 
-    plan_utils::MaskType maskType = plan_utils::MaskType::NO_MASK;
-    EXPECT_NO_THROW(maskType = classifyMask(builder));
-    EXPECT_EQ(maskType, plan_utils::MaskType::BOTTOM_RIGHT_CAUSAL);
+    plan_utils::ResolvedMask mask;
+    EXPECT_NO_THROW(mask = resolveMaskOf(builder));
+    EXPECT_EQ(mask.type, plan_utils::MaskType::SLIDING_WINDOW);
+    EXPECT_EQ(mask.left, 64);
+    EXPECT_EQ(mask.right, 0);
+    EXPECT_FALSE(mask.topLeft);
 }
 
 // Modern bounds-trio path (no deprecated boolean set). An unset bound is treated

dnn-providers/hip-kernel-provider/src/tests/engines/asm_sdpa_engine/TestSdpaFwdPlanBuilder.cpp

@@ -804,9 +804,10 @@ TEST_F(TestSdpaFwdPlanBuilder, GetMaxWorkspaceSizeCalculatesCorrectly)
 // =============================================================================
 //
 // These tests exercise the shared mask-precedence policy directly through
-// plan_utils::getMaskType rather than through isApplicable. When a deprecated
-// causal boolean is set it takes precedence over the modern bounds trio; only
-// setting both deprecated booleans at once throws. The policy is
+// plan_utils::getMaskType rather than through isApplicable. A deprecated causal
+// boolean fixes the diagonal and its alignment but keeps a real left_bound
+// (a causal sliding window); only setting both deprecated booleans at once
+// throws. The policy is
 // hardware-agnostic (it runs before any device dispatch and independent of the
 // kernel registry), so testing the helper keeps the assertions meaningful on
 // any device — including this gfx950 box. Driving the policy through
@@ -934,22 +935,24 @@ TEST_F(TestSdpaFwdPlanBuilder, IsApplicableRejectsCausalMaskAndBottomRightSetTog
     EXPECT_THROW(classifyMask(builder), hipdnn_plugin_sdk::HipdnnPluginException);
 }
 
-TEST_F(TestSdpaFwdPlanBuilder, IsApplicablePrefersCausalMaskOverWindowBounds)
+TEST_F(TestSdpaFwdPlanBuilder, CausalMaskWithLeftBoundIsSlidingWindow)
 {
     using namespace hipdnn_flatbuffers_sdk::data_objects;
 
-    // causal_mask=true takes precedence over the bounds trio, even though the
-    // bounds describe a sliding window (left=64, right=64): result is causal.
+    // causal_mask=true plus left_bound is a causal sliding window (cuDNN's
+    // set_causal_mask(true).set_sliding_window_length(n)), not plain causal.
+    // No forward kernel serves a window, so the engine must decline it rather
+    // than widen the window to the full causal triangle.
     auto builder = createSdpaFwdGraphWithMask(
         /*causalMask=*/true,
         /*causalMaskBottomRight=*/false,
         flatbuffers::Optional<int64_t>(64),
-        flatbuffers::Optional<int64_t>(64),
+        flatbuffers::nullopt,
         DiagonalAlignment::TOP_LEFT);
 
     plan_utils::MaskType maskType = plan_utils::MaskType::NO_MASK;
     EXPECT_NO_THROW(maskType = classifyMask(builder));
-    EXPECT_EQ(maskType, plan_utils::MaskType::TOP_LEFT_CAUSAL);
+    EXPECT_EQ(maskType, plan_utils::MaskType::SLIDING_WINDOW);
 }
 
 TEST_F(TestSdpaFwdPlanBuilder, IsApplicablePrefersBottomRightCausalOverTopLeftBounds)
@@ -989,23 +992,21 @@ TEST_F(TestSdpaFwdPlanBuilder, IsApplicableAcceptsConsistentCausalMaskAndBounds)
     EXPECT_EQ(maskType, plan_utils::MaskType::TOP_LEFT_CAUSAL);
 }
 
-TEST_F(TestSdpaFwdPlanBuilder, IsApplicablePrefersBottomRightCausalOverWindowBounds)
+TEST_F(TestSdpaFwdPlanBuilder, BottomRightCausalMaskWithLeftBoundIsSlidingWindow)
 {
     using namespace hipdnn_flatbuffers_sdk::data_objects;
 
-    // causal_mask_bottom_right=true takes precedence over the bounds trio, even
-    // though a symmetric sliding window (left=64, right=64) would derive
-    // SLIDING_WINDOW: result is bottom-right causal.
+    // Same rule for causal_mask_bottom_right.
     auto builder = createSdpaFwdGraphWithMask(
         /*causalMask=*/false,
         /*causalMaskBottomRight=*/true,
         flatbuffers::Optional<int64_t>(64),
-        flatbuffers::Optional<int64_t>(64),
+        flatbuffers::nullopt,
         DiagonalAlignment::BOTTOM_RIGHT);
 
     plan_utils::MaskType maskType = plan_utils::MaskType::NO_MASK;
     EXPECT_NO_THROW(maskType = classifyMask(builder));
-    EXPECT_EQ(maskType, plan_utils::MaskType::BOTTOM_RIGHT_CAUSAL);
+    EXPECT_EQ(maskType, plan_utils::MaskType::SLIDING_WINDOW);
 }
 
 // Modern bounds-trio path (no deprecated boolean set). An unset bound is treated