[PATCH] D37216: [SROA] propagate !range metadata when moving loads

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:168
 
+/// \brief Semi-open interval of instructions that are guaranteed to execute.
+class GuaranteedExecutionRange {
----------------
  ... guaranteed to execute once the first one executes.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:209
+  /// were created - even interesting instructions. The code will handle
+  /// the renumbering correctly. However, the still-present interesting
+  /// instructions must remain in the same order.
----------------
How is that renumbering handled correctly?

Also, there's some assumption here that we don't insert new instructions that would break apart an existing internal? (I'm guessing not, as that seems like it would be a semantics-changing transformation, and we don't do that here, but we should document the assumptions here).



================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:232
+      if (isInterestingInstruction(&BBI)) {
+        DenseMap<const Instruction *, unsigned>::iterator It = InstNumbers.find(&BBI);
+        assert(It != InstNumbers.end() &&
----------------
Line too long (and you should just use auto here).


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:372
       : Allocas(Allocas.begin(), Allocas.end()), DT(DT),
+	DL(DT.getRoot()->getParent()->getParent()->getDataLayout()),
         DIB(*DT.getRoot()->getParent()->getParent(), /*AllowUnresolved*/ false),
----------------
Odd indentation.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:420
+				       const DataLayout &DL,
+                                       LargeBlockInfo &LBI,
+				       AssumptionCache *AC) {
----------------
Odd indentation.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:475-476
                                      LargeBlockInfo &LBI, DominatorTree &DT,
+				     const DataLayout &DL,
                                      AssumptionCache *AC) {
   StoreInst *OnlyStore = Info.OnlyStore;
----------------
Odd indentation.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:578
                                      DominatorTree &DT,
+				     const DataLayout &DL,
                                      AssumptionCache *AC) {
----------------
Odd indentation.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:998
                                 RenamePassData::ValVector &IncomingVals,
+				LargeBlockInfo &LBI,
                                 std::vector<RenamePassData> &Worklist) {
----------------
Odd indentation.


https://reviews.llvm.org/D37216