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

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 423986.
fhahn added a comment.

I think this needs another look. AFAICT the original version was causing some mis-compiles, because *if* the object we store to is function local (alloca), we sink the store even if the store is not guarnateed to execute (tests added in 5e54a413de1f803 <https://reviews.llvm.org/rG5e54a413de1f803718816dba35a59fbc4c7ed082>).

I updated the patch to introduce the load, unless the the store is guaranteed to execute. If the load is introduced, the writeonly attribute is removed. This isn't ideal, because the attribute could have knock-on effects for functions calling the modified function.

Maybe a better solution would be to clarify the wording for writeonly in langref to be clear that the attribute only considers writes to memory visible to its callers. Then we can retain writeonly even if loads to local objects are introduced. I doubt considering reads to function-local objects for writeonly adds any value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123473

Files:
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/test/Transforms/LICM/scalar-promote.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123473.423986.patch
Type: text/x-patch
Size: 6788 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220420/2dc22a6f/attachment.bin>