[PATCH] D29668: Elide argument copies during instruction selection

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

More comments...



================
Comment at: include/llvm/Target/TargetCallingConv.h:48
     unsigned IsInConsecutiveRegs : 1;
+    unsigned IsCopyElisionCandidate : 1; ///< Argument copy elision candidate
 
----------------
This isn't really that the argument is an elision candidate, it means the argument's copy is *elided* and you transform based on this.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8106
+
+    // Check if the destination is a static alloca.
+    const Value *Dst = SI->getPointerOperand()->stripPointerCasts();
----------------
What if the value being stored is the address of a static alloca? We don't mark it as clobbered above because we're in the store instruction case.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8132-8134
+    // Mark this alloca and store for argument copy elision.
+    *Info = StaticAllocaInfo::Elidable;
+    ArgCopyElisionCandidates.insert({Arg, {AI, SI}});
----------------
Would it be worth stopping the scan when you're out of arguments?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8278-8279
       Flags.setOrigAlign(OriginalAlignment);
+      if (ArgCopyElisionCandidates.count(&Arg))
+        Flags.setCopyElisionCandidate();
 
----------------
You erase things from the set below but don't clear the flags.... I assume this can't manifest because of how the flag is currently used, but it seems surprising at the least. Maybe sink this to the below code where you're going to do the elision?


https://reviews.llvm.org/D29668