[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