[PATCH] D38278: [CGP] Make optimizeMemoryInst capable of handling multiple AddrModes

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2680
+  Value *OriginalValue = nullptr;
+  User *AddrModeUser = nullptr;
+
----------------
I cannot understand why you need a AddrModeUser. The only one use of this field I see is for checking that user of AddrMode is Phi or Select.

However according to usage of AddressingModeCombiner AddrModeUser cannot be anything else.

Do I miss anything?


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:3364
+    // which just duplicates what's already there.
+    if (!NewAddrMode.isTrivial())
+      AllAddrModesTrivial = false;
----------------
if AllAddrModesTrivial is false, you do not need to compute isTrivial. So you can re-write it as

  AllAddrModesTrivial = AllAddrModesTrivial && NewAddrMode.isTrivial();

Or alternatively:
  if (AllAddrModesTrivial)
    AllAddrModesTrivial = NewAddrMode.isTrivial();


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4566
 
-    if (!AddrModeFound) {
-      AddrModeFound = true;
-      AddrMode = NewAddrMode;
-      continue;
+    if (!AddrModes.addNewAddrMode(NewAddrMode)) {
+      break;
----------------
Redundant {}


https://reviews.llvm.org/D38278