[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:219
+
+static inline size_t alignUp(size_t Value, uint Alignment) {
+  return (Value + Alignment - 1) & ~(Alignment - 1);
----------------
MathExtras already has alignTo


================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:390
+      StoreInst *StBuff = Builder.CreateStore(toStore, PtrToStore);
+      LLVM_DEBUG(dbgs() << "inserting store to printf buffer:\n"
+                        << *StBuff << '\n');
----------------
Remove this newline 


================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:392
+                        << *StBuff << '\n');
+      PtrToStore = Builder.CreateGEP(
+          Builder.getInt8Ty(), PtrToStore,
----------------
You can use one of the CreateConstGEPs. I also suspect this can be inbounds 


================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:398
+          "PrintBuffNextPtr");
+      LLVM_DEBUG(dbgs() << "inserting gep to the printf buffer:\n"
+                        << *PtrToStore << '\n');
----------------
Remove this newline


================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:455
+        {ArgSize, ConstantInt::get(Int1Ty, IsConstFmtStr ? 1 : 0, false),
+         ConstantInt::get(Int1Ty, 0, false)});
+    Builder.CreateStore(valueToStore, Ptr);
----------------
There's a getFalse 


================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:480
+                          Ptr);
+      Ptr = Builder.CreateGEP(Int8Ty, Ptr, {ConstantInt::get(Int32Ty, 8)});
+    } else {
----------------
CreateConstGEP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150427