[PATCH] D122130: Verify parameter alignment attribute

LuoYuanke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 21 23:20:56 PDT 2022


LuoYuanke added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:1819
+        // ISD::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
----------------
skan wrote:
> The `Align(1<<15)` and comments here are duplicated in D121898,  I suggest that you add `Align(1<<15)` as a const static member of `Verifier`, and move the comments to the position of the definition.  The we can avoid duplicated code.
Good suggestion. I apply your suggestion in https://reviews.llvm.org/D121898. Will update this patch after D121898 land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122130



More information about the llvm-commits mailing list