[PATCH] D46662: [X86] condition branches folding for three-way conditional codes

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:275
+  while (PredMBB) {
+    TargetMBBInfo *PredMBBInfo = &*(MBBInfos.find(PredMBB)->second.get());
+    if (!PredMBBInfo || PredMBBInfo->SrcReg != MBBInfo->SrcReg)
----------------
&* isn't needed


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:349
+                                     MachineBasicBlock *NewDest) {
+  TargetMBBInfo *MBBInfo = &*(MBBInfos.find(MBB)->second.get());
+  MachineInstr *BrMI;
----------------
&* isn't needed


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:412
+  X86::CondCode CC;
+  TargetMBBInfo *MBBInfo = &*(MBBInfos.find(&MBB)->second.get());
+  MachineBasicBlock *TargetMBB = MBBInfo->TBB;
----------------
I don't think this "&*" is neededed. get() should return a pointer. you shouldn't need to dereference that and take the address of it.


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:414
+  MachineBasicBlock *TargetMBB = MBBInfo->TBB;
+  assert(MBBInfo);
+  BranchProbability TargetProb = MBPI->getEdgeProbability(&MBB, MBBInfo->TBB);
----------------
This assert doesn't really accomplish anything. MBBInfo was already dereferenced on the line before so we crashed before we got to the assert.


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:419
+  MachineBasicBlock *PredMBB = BranchPath.front();
+  TargetMBBInfo *PredMBBInfo = &*(MBBInfos.find(PredMBB)->second.get());
+  assert(PredMBBInfo);
----------------
&* isn't needed


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:430
+  MachineBasicBlock *RootMBB = BranchPath.back();
+  TargetMBBInfo *RootMBBInfo = &*(MBBInfos.find(RootMBB)->second.get());
+  assert(RootMBBInfo);
----------------
&* isn't needed.


================
Comment at: lib/Target/X86/X86CondBrFolding.cpp:548
+      MachineBasicBlock *PMBB = *I;
+      TargetMBBInfo *PMBBInfo = &*(MBBInfos.find(PMBB)->second.get());
+      LLVM_DEBUG(dbgs() << "Path MBB (" << Index << " of " << BranchPath.size()
----------------
&* isn't needed


https://reviews.llvm.org/D46662