[PATCH] D136169: [AMDGPU] Avoid SCC clobbering before S_CSELECT_B32

[Alexander via Phabricator via llvm-commits]

alex-t marked an inline comment as done.
alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2238
+              .addReg(FrameReg)
+              .addImm(Offset);
+          BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_BITCMP1_B32))
----------------
sebastian-ne wrote:
> alex-t wrote:
> > sebastian-ne wrote:
> > > Maybe it makes sense to add an assert, that the least significant bit of `Offset` is indeed 0? (here and for s_subb below)
> > As far as I know, the flat scratch offset is always dword-aligned. If it is not we've got the UB anyway. So, the assert does not help.
> Sure, an assert doesn’t help correctness. But if at some point in the future, we encounter a case where the offset is not aligned – maybe due to a bug elsewhere – having an assert makes it easy to find why some memory got corrupted.
> Without an assert, it will surely take hours to find out what is going on.
The assert which is currently added only ensures that the offset is even.
This is enough to make sure we have the correct arithmetics here. Although to ensure correct alignment it should be 
```
!(Offset & flat_scratch_min_align)
```
I am not sure if it makes sense but  maybe there should be another error message?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136169