[PATCH] D121898: [Verifier] Verify parameter alignment.

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:3149-3157
+    Align ABIAlign = DL.getABITypeAlign(Ty);
+    // ArgFlagsTy::MemAlign only have 4 bits for alignment, so the
+    // alignment size should not exceed 2^15. Since encode(Align)
+    // would plus the shift value by 1, the alignment size should
+    // not exceed 2^14, otherwise it can NOT be properly lowered
+    // in backend.
+    Align MaxAlign(1 << 15);
----------------
pengfei wrote:
> The variables are only used in Assert, should add `#ifndef NDEBUG` to them? Surprisingly I didn't find an example in this file.
The Assert is defined as below. Let me check if it will fail in release build.

```
#define Assert(C, ...) \
  do { if (!(C)) { CheckFailed(__VA_ARGS__); return; } } while (false)
```


================
Comment at: llvm/test/Verifier/param-ret-align.ll:1
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
----------------
pengfei wrote:
> Can we merge them to one file?
Probably not. llvm-as would exit on the first failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121898/new/

https://reviews.llvm.org/D121898