================
@@ -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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to