[clang] [HLSL] add loop unroll (PR #93879)

Farzon Lotfi via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 24 17:19:33 PDT 2024


================
@@ -584,6 +585,39 @@ static Attr *handleOpenCLUnrollHint(Sema &S, Stmt *St, const ParsedAttr &A,
   return ::new (S.Context) OpenCLUnrollHintAttr(S.Context, A, UnrollFactor);
 }
 
+static Attr *handleHLSLLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                                    SourceRange Range) {
+
+  if (A.getSemanticSpelling() == HLSLLoopHintAttr::Spelling::Microsoft_loop &&
+      !A.checkAtMostNumArgs(S, 0))
+    return nullptr;
+
+  unsigned UnrollFactor = 0;
+  if (A.getNumArgs() == 1) {
+
+    if (A.isArgIdent(0)) {
+      S.Diag(A.getLoc(), diag::err_attribute_argument_type)
+          << A << AANT_ArgumentIntegerConstant << A.getRange();
+      return nullptr;
+    }
+
+    Expr *E = A.getArgAsExpr(0);
+
+    if (S.CheckLoopHintExpr(E, St->getBeginLoc(),
+                            /*AllowZero=*/false))
+      return nullptr;
+
+    std::optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(S.Context);
+    // CheckLoopHintExpr handles non int const cases
+    assert(ArgVal != std::nullopt && "ArgVal should be an integer constant.");
+    int Val = ArgVal->getSExtValue();
+    // CheckLoopHintExpr handles negative and zero cases
+    assert(Val > 0 && "Val should be a positive integer greater than zero.");
----------------
farzonl wrote:

Good question.  You are correct it is not needed, I added it as a form of future proofing. Let me explain my thinking.

`CheckLoopHintExpr` is the sema checks initially created for clang loop unrolling. We are just leveraging it. There are  typically two ways of enforcing a contract on expected behavior, unit tests and asserts. I typically like to include  both asserts and unit tests.  of the two asserts are my preference because they encode your expectations in the code base and are not tangential to it like unit tests.

As a thought experiment lets imagine a change to `CheckLoopHintExpr`.  This is unlikely to happen, but  for the sake of arguement lets say the behavior of `CheckLoopHintExpr` changes to one day support non constant ints. Then the nullptr would not return and we would get a fall through.  We  want alerts to this regression in behavior on the HLSL portion and asserts are my prefered way of knowing what happend  for the reasons mentioned above but also because you get a callstack.  I use to write a bunch of c# code, we had code contracts to define expectations. Thats kind of what i'm trying to do here. 

https://github.com/llvm/llvm-project/pull/93879


More information about the cfe-commits mailing list