[PATCH] D152222: [RISCV] Fix the num of chain SDNode introduced in 9e0f9f113248093e737c4cf5450f0a3c2bcd90ba

Mon Jun 5 22:59:36 PDT 2023

craig.topper added a comment.

In D152222#4398167 <https://reviews.llvm.org/D152222#4398167>, @zixuan-wu wrote:

>> It’s not legal to replace the chain result of the original node wasn’t deleted. We would need to insert a TokenFactor to collect the chains from the new and old load and replace all uses of the old load’s chain with the TokenFactor result.
>
> OK, using TokenFactor is more formal, and I agree with it. But it's another issue to enhance the code, we still need think about load update load.
>
>> It’s also not legal to duplicate a volatile load. I suspect we didn’t check that.
>
> I aggree.
>
>> I think we should disable the load update case in isLegalToFold or isProfitableToFold.
>
> Why is load update not profitable or legal? I don't see any difference with normal load.

We only fold normal loads if it removes the scalar load. So that's different than the load update which won't be removed.

It's not legal if we don't add the TokenFactor and fix the volatile issue. Making it not legal/profitable avoids those issues.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152222