[PATCH] D127858: [RISCV] Turn on SeparateConstOffsetFromGEPPass for RISC-V target

Wed Jun 22 03:34:57 PDT 2022

asb added a comment.

In D127858#3596240 <https://reviews.llvm.org/D127858#3596240>, @eklepilkina wrote:

>> Are you seeing any instances of worse code in any of the tests / benchmarks you're looking at?
>
> Yes, I ran almost all llvm test-suite benchmarks. There were some unstable benchmarks, but regressions on them weren't reproduced in the next runs. Do you have the cases with previous regressions you've found out?
>
>> Presumably the changes in D127727 <https://reviews.llvm.org/D127727> were motivated by such cases?
>
> Exactly, besides unstable cases we also found several regressions and my investigation mostly shows that the current pass originally modifies all GEPs althought sometimes it increase register pressure, so new startegy was added. Of course, it's possible I missed something.
>
> I'll be grateful if you provide some list of benchmarks that I should pay more attention.

I don't have anything in particular in mind. I'd just looked at turning it on, and found the resulting codegen story was more mixed than I hoped and so shelved it for the time being. It might be worth looking at diffing the generated assembly (either objdump output or .s from -save-temps) for your benchmarks. Regressions in generated code quality that don't negatively impact benchmark results are obviously less worrying than those that do, but if this change is introducing more cases of obviously "bad" code, it's likely going to eat up time over the longer term as users report it or it's investigated as a potential performance issue.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127858