[PATCH] D72998: [IR] Attribute/AttrBuilder: use Value::MaximumAlignment magic constant

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: erichkeane, hfinkel, jdoerfert, gchatelet, courbet.
lebedev.ri added a project: LLVM.
Herald added subscribers: cfe-commits, hiraditya.
Herald added a project: clang.
lebedev.ri added a parent revision: D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation.

I initially encountered those assertions when trying to create
this IR `alignment` attribute from clang's `__attribute__((assume_aligned(imm)))`,
because until D72994 <https://reviews.llvm.org/D72994> there is no sanity checking for the value of `imm`.

But even then, we have `llvm::Value::MaximumAlignment` constant (which is `536870912`),
which is enforced for clang attributes, and then there are some other magical constant
(`0x40000000` i.e. `1073741824` i.e. `2 * 536870912`) in
`Attribute::getWithAlignment()`/`AttrBuilder::addAlignmentAttr()`.

I strongly suspect that `0x40000000` is incorrect,
and that also should be `llvm::Value::MaximumAlignment`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72998

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/lib/IR/Attributes.cpp


Index: llvm/lib/IR/Attributes.cpp
===================================================================
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -143,7 +143,7 @@
 }
 
 Attribute Attribute::getWithAlignment(LLVMContext &Context, Align A) {
-  assert(A <= 0x40000000 && "Alignment too large.");
+  assert(A <= llvm::Value::MaximumAlignment && "Alignment too large.");
   return get(Context, Alignment, A.value());
 }
 
@@ -1521,7 +1521,7 @@
   if (!Align)
     return *this;
 
-  assert(*Align <= 0x40000000 && "Alignment too large.");
+  assert(*Align <= llvm::Value::MaximumAlignment && "Alignment too large.");
 
   Attrs[Attribute::Alignment] = true;
   Alignment = Align;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1627,7 +1627,7 @@
     }
 
     // Alignment calculations can wrap around if it's greater than 2**29.
-    unsigned MaximumAlignment = 536870912;
+    unsigned MaximumAlignment = 536870912; // See llvm::Value::MaximumAlignment
     if (I > MaximumAlignment)
       Diag(CI.getLoc(), diag::warn_assume_aligned_too_great)
           << CI.getRange() << MaximumAlignment;
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4486,7 +4486,8 @@
         }
 
         // Alignment calculations can wrap around if it's greater than 2**29.
-        unsigned MaximumAlignment = 536870912;
+        unsigned MaximumAlignment =
+            536870912; // See llvm::Value::MaximumAlignment
         if (I > MaximumAlignment)
           Diag(Arg->getExprLoc(), diag::warn_assume_aligned_too_great)
               << Arg->getSourceRange() << MaximumAlignment;
@@ -6215,7 +6216,7 @@
              << Arg->getSourceRange();
 
     // Alignment calculations can wrap around if it's greater than 2**29.
-    unsigned MaximumAlignment = 536870912;
+    unsigned MaximumAlignment = 536870912; // See llvm::Value::MaximumAlignment
     if (Result > MaximumAlignment)
       Diag(TheCall->getBeginLoc(), diag::warn_assume_aligned_too_great)
           << Arg->getSourceRange() << MaximumAlignment;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72998.238984.patch
Type: text/x-patch
Size: 2286 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200119/ac359659/attachment-0001.bin>