[PATCH] D80422: Enable `align <n>` to be used in intrinsic definitions.

[Michael Liao via Phabricator via llvm-commits]

hliao marked 5 inline comments as done.
hliao added inline comments.
Herald added a subscriber: wuzish.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:78
+  int ArgNo = argNo;
+  int Align = align;
+}
----------------
jdoerfert wrote:
> Nit: maybe `uint64_t` for the align to match other places, though it will work fine as long as `int` is 32 bit (or more).
`int` in td is represented as `int64_t` internally.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:173
   Intrinsic<[LLVMQualPointerType<llvm_i8_ty, 4>], [],
-  [IntrNoMem, IntrSpeculatable]>;
+  [Align<-1, 4>, IntrNoMem, IntrSpeculatable]>;
 
----------------
jdoerfert wrote:
> I think -1 is in principle the right thing here *but* we should try to make it a variable with a proper name, e.g. `ReturnIndex`.
introduce `RetIndex` and `ArgIndex<n>` to specifiy that attribute indices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80422