[PATCH] D126576: [RISCV] Add custom isel for (add X, imm) used by load/stores.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D126576#3547631 <https://reviews.llvm.org/D126576#3547631>, @reames wrote:

> In D126576#3544185 <https://reviews.llvm.org/D126576#3544185>, @craig.topper wrote:
>
>> In D126576#3544140 <https://reviews.llvm.org/D126576#3544140>, @reames wrote:
>>
>>> Just a thought on an alternate approach.  Feel free to ignore.
>>>
>>> Given:
>>> addi a2, a2, Low12C
>>> add a2, a0, a2
>>>
>>> Is there any reason we shouldn't canonicalize toward:
>>> add a2, a0, a2
>>> addi a2, a2, Low12C
>>>
>>> That is, try to push the add with immediate towards the users?  (Assume a one use restriction on the original addi.)
>>>
>>> This isn't an optimization per se, but if we could treat it as a canonicalization, I think it simplifies the address matching problem significantly.
>>
>> Where do you propose to canonicalize it? Another peephole between isel and doPeepholeLoadStoreADDI?
>
> It turns out I had a wrong mental model on SDAG handling of constants.  I had been thinking we'd expanded the constant materialization sequences during legalization; looking at an example with debug output, this turns out not to be the case.  As such, my assumption that this was just a DAGCombine rule is off base.
>
> Given that, yeah, the structure would seem to require a separate peephole between ISEL and the current address matching.  ("between" might be slightly the wrong word here as we could probably fixed point the later two.)

I'm not sure it would significantly simplify much over this patch. I would still want to check the memory folding opportunity before moving the ADDI across the ADD. Some targets may support LUI+ADDI macrofusion so we shouldn't split those up unless the ADDI is guaranteed to be removed.

Doing it as part of isel means we strip the lower bits off before constant materialization instead of needing to pattern match the LUI+ADDIW in a peephole. Though I guess maybe that could be avoided if we used LUI+ADDI instead of LUI+ADDIW when possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126576