Address review comments

clang/include/clang/Basic/Attr.td

@@ -4282,10 +4282,17 @@ def PreferredType: InheritableAttr {
 }
 
 def CodeAlign: StmtAttr {
-  let Spellings = [CXX11<"clang", "code_align">,
-                   C23<"clang", "code_align">];
+  let Spellings = [Clang<"code_align">];
   let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt],
                               ErrorDiag, "'for', 'while', and 'do' statements">;
-  let Args = [ExprArgument<"NExpr">];
+  let Args = [ExprArgument<"Alignment">];
   let Documentation = [CodeAlignAttrDocs];
+  let AdditionalMembers = [{
+    static int getMinValue() {
+      return 1;
+    }
+    static int getMaxValue() {
+      return 4096;
+    }
+  }];
 }

clang/include/clang/Basic/AttrDocs.td

@@ -7421,12 +7421,11 @@ def CodeAlignAttrDocs : Documentation {
   let Category = DocCatVariable;
   let Heading = "clang::code_align";
   let Content = [{
-The ``clang::code_align(N)`` attribute applies to a loop and it specifies the
-byte alignment for a loop. The attribute accepts a positive integer constant
+The ``clang::code_align(N)`` attribute applies to a loop and specifies the byte
+alignment for a loop. The attribute accepts a positive integer constant
 initialization expression indicating the number of bytes for the minimum
-alignment boundary. Its value must be a power of 2, between 1 and 4096, such as
-1, 2, 4, 8, and so on. This attribute sets ``llvm.loop.align`` loop metadata
-when it applies on a loop statement.
+alignment boundary. Its value must be a power of 2, between 1 and 4096
+(inclusive).
 
 .. code-block:: c++
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10028,8 +10028,8 @@ def warn_missing_case_for_condition :
   Warning<"no case matching constant switch condition '%0'">;
 def err_loop_attr_duplication : Error<
   "duplicate loop attribute %0">;
-def err_attribute_argument_not_power_of_two : Error<
-  "%0 attribute argument must be a constant power of two greater than zero">;
+def err_attribute_power_of_two_in_range : Error<
+  "%0 attribute must be a constant power of two between %1 and %2 inclusive">;
 
 def warn_def_missing_case : Warning<"%plural{"
   "1:enumeration value %1 not explicitly handled in switch|"

clang/lib/CodeGen/CGLoopInfo.cpp

@@ -797,15 +797,12 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
     }
   }
 
-  // Translate 'loop attributes' arguments to equivalent Attr enums.
-  // It's being handled separately from LoopHintAttrs not to support
-  // legacy GNU attributes and pragma styles.
-  //
+  // Identify loop attribute 'code_align' from Attrs.
   // For attribute code_align:
   // n - 'llvm.loop.align i32 n' metadata will be emitted.
   for (const auto *A : Attrs) {
     if (const auto *CodeAlign = dyn_cast<CodeAlignAttr>(A)) {
-      const auto *CE = cast<ConstantExpr>(CodeAlign->getNExpr());
+      const auto *CE = cast<ConstantExpr>(CodeAlign->getAlignment());
       llvm::APSInt ArgVal = CE->getResultAsAPSInt();
       setCodeAlign(ArgVal.getSExtValue());
     }

clang/lib/CodeGen/CGLoopInfo.h

@@ -79,7 +79,7 @@ struct LoopAttributes {
   /// Value for llvm.loop.pipeline.iicount metadata.
   unsigned PipelineInitiationInterval;
 
-  /// Value for 'llvm.loop.align' loop metadata.
+  /// Value for 'llvm.loop.align' metadata.
   unsigned CodeAlign;
 
   /// Value for whether the loop is required to make progress.
@@ -285,7 +285,7 @@ class LoopInfoStack {
     StagedAttrs.PipelineInitiationInterval = C;
   }
 
-  /// Set the CodeAlign for the next loop pushed.
+  /// Set value of code align for the next loop pushed.
   void setCodeAlign(unsigned C) { StagedAttrs.CodeAlign = C; }
 
   /// Set no progress for the next loop pushed.

clang/lib/Sema/SemaStmtAttr.cpp

@@ -331,17 +331,13 @@ CodeAlignAttr *Sema::BuildCodeAlignAttr(const AttributeCommonInfo &CI,
       return nullptr;
     E = Res.get();
 
-    // This attribute requires a strictly positive value.
-    if (ArgVal <= 0) {
-      Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
-          << CI << /*positive*/ 0;
-      return nullptr;
-    }
-
-    // This attribute requires a single constant power of two greater than zero.
-    if (!ArgVal.isPowerOf2()) {
-      Diag(E->getExprLoc(), diag::err_attribute_argument_not_power_of_two)
-          << CI;
+    int align_value = ArgVal.getSExtValue();
+    if (align_value < CodeAlignAttr::getMinValue() ||
+        align_value > CodeAlignAttr::getMaxValue() ||
+        !ArgVal.isPowerOf2()) {
+      Diag(CI.getLoc(), diag:: err_attribute_power_of_two_in_range)
+          << CI << CodeAlignAttr::getMinValue()
+	  << CodeAlignAttr::getMaxValue();
       return nullptr;
     }
   }
@@ -360,15 +356,15 @@ template <typename LoopAttrT>
 static void
 CheckForDuplicateLoopAttribute(Sema &S,
                                const SmallVectorImpl<const Attr *> &Attrs) {
-  const LoopAttrT *LoopAttr = nullptr;
-
+  const Attr *A = nullptr;
   for (const auto *I : Attrs) {
-    if (LoopAttr && isa<LoopAttrT>(I)) {
-      // Cannot specify same type of attribute twice.
-      S.Diag(I->getLocation(), diag::err_loop_attr_duplication) << LoopAttr;
+    if (isa<LoopAttrT>(I)) {
+      if (A) {
+        // Cannot specify same type of attribute twice.
+        S.Diag(I->getLocation(), diag::err_loop_attr_duplication) << A;
+       }
+       A = I;
     }
-    if (isa<LoopAttrT>(I))
-      LoopAttr = cast<LoopAttrT>(I);
   }
 }
 

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -1907,7 +1907,7 @@ const AlwaysInlineAttr *TemplateInstantiator::TransformStmtAlwaysInlineAttr(
 
 const CodeAlignAttr *
 TemplateInstantiator::TransformCodeAlignAttr(const CodeAlignAttr *CA) {
-  Expr *TransformedExpr = getDerived().TransformExpr(CA->getNExpr()).get();
+  Expr *TransformedExpr = getDerived().TransformExpr(CA->getAlignment()).get();
   return getSema().BuildCodeAlignAttr(*CA, TransformedExpr);
 }
 

clang/test/CodeGen/code_align.c

@@ -1,7 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck -check-prefix=CHECK-C %s
-// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK-C,CHECK-CPP
-
-// Add CodeGen tests for Loop attribute: [[clang::code_align)]].
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -x c %s %s -o - | FileCheck -check-prefix=CHECK-C %s
+// RUN: %clang_cc1 -fsyntax-only -emit-llvm -x c++ -std=c++11 %s -o - | FileCheck %s --check-prefixes CHECK-C,CHECK-CPP
 
 // CHECK-C: br label %for.cond, !llvm.loop ![[MD_FP:[0-9]+]]
 // CHECK-C: br label %while.cond, !llvm.loop ![[MD_FP_1:[0-9]+]]

clang/test/Sema/code_align.c

@@ -1,7 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,c-local %s
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,cpp-local -pedantic -x c++ -std=c++11 %s 
-
-// Add diagnostics tests for Loop attribute: [[clang::code_align()]].
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c-local -x c %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cpp-local -pedantic -x c++ -std=c++11 %s
 
 void foo() {
   int i;
@@ -28,11 +26,11 @@ void bar(int);
 #endif
 void foo1(int A)
 {
-  // expected-error@+1 {{'code_align' attribute requires a positive integral compile time constant expression}}
+  // expected-error@+1 {{'code_align' attribute must be a constant power of two between 1 and 4096 inclusive}}
   [[clang::code_align(0)]]
   for(int I=0; I<128; ++I) { bar(I); }
 
-  // expected-error@+1{{'code_align' attribute requires a positive integral compile time constant expression}}
+  // expected-error@+1{{'code_align' attribute must be a constant power of two between 1 and 4096 inclusive}}
   [[clang::code_align(-4)]]
   for(int I=0; I<128; ++I) { bar(I); }
 
@@ -69,10 +67,14 @@ void foo1(int A)
   [[clang::code_align(64)]]
   for(int I=0; I<128; ++I) { bar(I); }
 
-  // expected-error@+1 {{'code_align' attribute argument must be a constant power of two greater than zero}}
+  // expected-error@+1 {{'code_align' attribute must be a constant power of two between 1 and 4096 inclusive}}
   [[clang::code_align(7)]]
   for(int I=0; I<128; ++I) { bar(I); }
 
+  // expected-error@+1 {{'code_align' attribute must be a constant power of two between 1 and 4096 inclusive}}
+  [[clang::code_align(5000)]]
+  for(int I=0; I<128; ++I) { bar(I); }
+
 #if __cplusplus >= 201103L
   // cpp-local-error@+5 {{expression is not an integral constant expression}}
   // cpp-local-note@+4 {{function parameter 'A' with unknown value cannot be used in a constant expression}}
@@ -112,13 +114,14 @@ void code_align_dependent() {
   [[clang::code_align(B)]]
   for(int I=0; I<128; ++I) { bar(I); }
 
-  // cpp-local-error@+1{{'code_align' attribute requires a positive integral compile time constant expression}}
+  // cpp-local-error@+2{{'code_align' attribute must be a constant power of two between 1 and 4096 inclusive}}
+  // cpp-local-note@#neg-instantiation {{in instantiation of function template specialization}}
   [[clang::code_align(D)]]
   for(int I=0; I<128; ++I) { bar(I); }
 }
 
 int main() {
-  code_align_dependent<8, 16, 32, -10>(); // cpp-local-note{{in instantiation of function template specialization 'code_align_dependent<8, 16, 32, -10>' requested here}}
+  code_align_dependent<8, 16, 32, -10>(); // #neg-instantiation
   return 0;
 }
 #endif

clang/test/Sema/code_align_ast.c

@@ -1,9 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -ast-dump -verify %s | FileCheck -check-prefix=CHECK-C %s
-// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -ast-dump %s | FileCheck %s --check-prefixes CHECK-C,CHECK-CPP
-
-// expected-no-diagnostics
-
-// Add AST tests for Loop attribute: [[clang::code_align()]].
+// RUN: %clang_cc1 -fsyntax-only -ast-dump -verify -x c %s | FileCheck -check-prefix=CHECK-C %s
+// RUN: %clang_cc1 -fsyntax-only -ast-dump -x c++ -std=c++11 %s | FileCheck %s --check-prefixes CHECK-C,CHECK-CPP
 
 void bar(int);
 // CHECK-C: FunctionDecl{{.*}}code_align 'void ()'