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

Hiroshi Inoue via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 26 23:43:00 PDT 2017

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? 


More information about the llvm-commits mailing list