[PATCH] D123473: [LICM] Only create load in pre-header when promoting load.

Nuno Lopes via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 2 04:01:20 PDT 2022


nlopes added a comment.

In D123473#3484702 <https://reviews.llvm.org/D123473#3484702>, @reames wrote:

> I don't think we actually want to do that.  Instead, I think we're making a mistake by considering the load to be immediate UB as opposed to simply returning poison in this case.

You've a point. My concern is that it's surprising to have LLVM introducing a load in a function that's not supposed to do any load. I was thinking about I/O, but those accesses should be volatile. For kernel code it may not be ok, but probably they also use volatile to prevent the compiler messing around instead of relying on `writeonly`.
I can give it a try. I will run Alive2 on the LLVM test suite and see if anything fails.
Nevertheless, as mentioned below, whether the load is UB or returns poison is orthogonal to this patch.

In D123473#3484702 <https://reviews.llvm.org/D123473#3484702>, @reames wrote:

> If we accept that semantic, then this patch becomes completely unnecessary.

Not true. The loop may not execute and thus you need the previous value. If the loaded value is poison, you still can't do the transformation,
Plus this patch removes useless loads; it's a perf win.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123473



More information about the llvm-commits mailing list