[PATCH] D35907: [WIP] Update TBAA information in stack coloring pass

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

I don't know much about our alias analysis infrastructure.

That said the description and changes seem reasonable. Please double check your getUnderlyingObject refactoring before submitting.



================
Comment at: include/llvm/CodeGen/MachineFunction.h:664
 
+  /// getMachineMemOperand - Allocate a new MachineMemOperand by copying
+  /// an existing one, replacing only AliasAnalysis information.
----------------
Don't repeat the method name in the doxygen comment (old code does it, new code shouldn't).


================
Comment at: include/llvm/CodeGen/ScheduleDAGInstrs.h:86
 
+  /// Helper function for tracking a Value to a reference to a known object.
+  void getUnderlyingObjects(const Value *V,
----------------
Explain more what the function does and less what it should be used for.

Also this isn't scheduler related, so at least move it to `MachineMemoryOperand` or better to ValueTracking.h (which already has a few variations of it, maybe you can even use one of those instead of roling another one).


================
Comment at: lib/CodeGen/StackColoring.cpp:40
 #include "llvm/CodeGen/PseudoSourceValue.h"
+#include "llvm/CodeGen/ScheduleDAGInstrs.h" // for getUnderlyingObjects
 #include "llvm/CodeGen/SlotIndexes.h"
----------------
Instead of documenting why you need to import a scheduling header here, just put the function into an apropriate header (as above) :)


================
Comment at: lib/CodeGen/StackColoring.cpp:1025
+                const AllocaInst *AI = dyn_cast_or_null<AllocaInst>(V);
+                if (AI && MergedAllocas.count(AI)) {
+                  MayModified = true;
----------------
Is `MergedAllocas` changing here? Looks like you can move the check further up outside this loop.


https://reviews.llvm.org/D35907