[PATCH] D83020: [AMDGPU] Avoid using s_cmpk when src0 is not register

Tue Jul 7 02:21:50 PDT 2020

ruiling marked an inline comment as done.
ruiling added inline comments.

================
Comment at: llvm/test/CodeGen/AMDGPU/cmp_shrink.mir:5
+# GCN: bb.0:
+# GCN-NOT: S_CMPK_GT_I32
+---
----------------
ruiling wrote:
> arsenm wrote:
> > ruiling wrote:
> > > arsenm wrote:
> > > > positive checks are more useful. Also you can just generate these checks. Can you reproduce this with an IR test too?
> > > will try positive check, how to generate the checks? could you give a little bit more info? The original test case that hit the issue is over-complex I think. Normally, a constant expression at IR level is easy to be optimized off by the middle-end. so I think a .mir test is enough for this issue.
> > So what is the context this appears? Why wasn't it optimized out?
> well I didn't carefully check the program yet to understand why the optimization algorithms in llvm fails to optimize the program. but I think that is another problem that worth a careful investigation. I will investigate and try to optimize it off later. But I think this patch can be merged, right? can anyone help to merge? I don't have commit access.
@arsenm  The issue occurs when running vulkancts again AMD open-source vulkan driver. I did a little more check, the test-case has ~40 BBs and lots of phi instructions which is later simplified and proved to be constant. And the problem may be because LLPC choose a subset of llvm optimization passes considering compilation time (https://github.com/GPUOpen-Drivers/llpc/blob/dev/lgc/patch/Patch.cpp#L223). I tried use LLVM standard set of passes, the constant was optimized off. I think the optimization passes for vulkan may need further tuning to reach a better trade-off between compile-time and quality of generated code.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83020