[PATCH] D93927: [ArgPromotion] Copy !range metadata for loads.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.
Herald added a subscriber: ormris.

Hey, I was just about to submit something like your first patch version here independently (and with the same bugs).

Anyway the latest version of the diff makes sense to me. I added some nitpicks but this is essentially good to go.



================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:318-325
+            bool isOrigLoadGuaranteedToExecute = true;
+            for (const Instruction &II : *BB) {
+              if (!isOrigLoadGuaranteedToExecute || &II == OrigLoad) {
+                break;
+              }
+              isOrigLoadGuaranteedToExecute =
+                  llvm::isGuaranteedToTransferExecutionToSuccessor(&II);
----------------
avoid check for isOrigLoadGuaranteedToExecute in every loop iteration.


================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:328-331
+              // Transfer the AA info too.
+              AAMDNodes AAInfo;
+              OrigLoad->getAAMetadata(AAInfo);
+              newLoad->setAAMetadata(AAInfo);
----------------
This already happens in line 312.


================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:333-337
+              newLoad->copyMetadata(*OrigLoad, {LLVMContext::MD_nontemporal,
+                                                LLVMContext::MD_nonnull,
+                                                LLVMContext::MD_dereferenceable,
+                                                LLVMContext::MD_noundef,
+                                                LLVMContext::MD_range});
----------------
How about using `llvm::copyMetadataForLoad` from `Transforms/Utils/Local.h` at a first glance it  seems to work fine even for loads in different functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93927