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

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs marked 3 inline comments as done.
inouehrs added inline comments.


================
Comment at: include/llvm/CodeGen/MachineFunction.h:665
+  /// getMachineMemOperand - Allocate a new MachineMemOperand by copying
+  /// an existing one, replacing only AliasAnalisys information.
+  /// MachineMemOperands are owned by the MachineFunction and need not be
----------------
grandinj wrote:
> AliasAnalisys -> AliasAnalysis
Fixed. Thanks!


================
Comment at: lib/CodeGen/MachineFunction.cpp:338
+    return new (Allocator)
+               MachineMemOperand(MachinePointerInfo(MMO->getValue(),
+                                                    MMO->getOffset()),
----------------
jtony wrote:
> If we add a temp variable for the first parameter and assign a different value to it according to the guard `if(MMO->getValue())`, won't the code be simpler? I am talking something like this:
> 
> ```
> const PseudoSourceValue *Value = MMO->getValue() ?  MMO->getValue() : MMO->getPseudoValue();
> 
> return new (Allocator)
>              MachineMemOperand(MachinePointerInfo(Value, MMO->getOffset()),
>                                MMO->getFlags(), MMO->getSize(),
>                                MMO->getBaseAlignment(), AAInfo, 
>                                MMO->getRanges(), MMO->getSyncScopeID(),
>                                MMO->getOrdering(), MMO->getFailureOrdering());
> }
> ```
I hope this looks better than the old code.
Since getValue() and getPseudoValue() return different types, I think this is easier to understand.


================
Comment at: lib/CodeGen/StackColoring.cpp:874
 
+/// FIXME: These two helper functions are copied from ScheduleDAGInstrs.cpp.
+///        To avoid the duplicated code.
----------------
jtony wrote:
> Can we get rid of the `static` key word for these two functions and  and put their declaration into a header and  just keep one copy of the source code? 
Done.


https://reviews.llvm.org/D35907