[PATCH] D60772: [AMDGPU] Add optional bounds checking for scratch accesses

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

In D60772#1470073 <https://reviews.llvm.org/D60772#1470073>, @arsenm wrote:

> I don't understand the problem being solved here. Who/what is this intended to benefit? An out of bounds access is going to be undefined.


Graphics APIs have "robust (buffer) access" settings in which out of bounds accesses must be guaranteed not to crash the program.



================
Comment at: lib/Target/AMDGPU/SIInsertScratchBounds.cpp:341-342
+    const MachineFrameInfo &FrameInfo = MF.getFrameInfo();
+    const int64_t ScratchSizeEstimate =
+      (int64_t) FrameInfo.estimateStackSize(MF);
+
----------------
critson wrote:
> arsenm wrote:
> > This isn't going to be strong enough to guarantee that there will never be an out of bounds access.
> > 
> > This approach also isn't going to work in a callable function, in the presence of calls, or variable sized stack objects (which I'm planning on implementing)
> On that basis do you have any opinions on the appropriate method for establishing stack size going forward?
You could emit the stack size as a TargetGlobalAddress referring to a special symbol, and ask PAL to fixup the relocations for that symbol at load time. Details would have to be figured out, but that works as a rough approach.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60772