[PowerPC] Fix the incorrect iterator inside peephole

[steven.zhang]

Summary

Instruction selection will insert nodes into the underlying list after the root node and iterating will thereby miss it. We should NOT assume that, the root node is the last element in the DAG nodelist.

[nemanjai]

I'll let Hal chime in here, but I'd prefer if we just refactor this code to use a range-for instead of this while loop (in all 3 cases).

[steven.zhang]

Hi, Nemanjai, if change it to the rangefor, we will walk the DAG in the reverse order(begin() -> end()), which result in two case failed. CodeGen/PowerPC/cmpb.ll and CodeGen/PowerPC/cmpb-ppc32.ll. It seems that, our implementation has some dependent on how we walk the DAG.

[hfinkel]

In D47437#1113899, @nemanjai wrote:

I'll let Hal chime in here, but I'd prefer if we just refactor this code to use a range-for instead of this while loop (in all 3 cases).

I would as well, but I don't think that we can here. The range over which we're iterating is being modified as we do it. In any case, if we're going to try to change this, I'd do it in two stages. First, this change (so we know that we're hitting everything). Then look at trying to simplify the iteration structure. This LGTM.

llvm/trunk/lib/Target/PowerPC/PPCISelDAGToDAG.cpp

do {
     Res = CurDAG->getSelect(dl, User->getValueType(0), Cond, TrueRes, FalseRes);
     N = User;
     ConstTrue = TrueRes;
     ConstFalse = FalseRes;
   } while (N->hasOneUse());
 }
 
 void PPCDAGToDAGISel::PreprocessISelDAG() {
-  SelectionDAG::allnodes_iterator Position(CurDAG->getRoot().getNode());
-  ++Position;
+  SelectionDAG::allnodes_iterator Position = CurDAG->allnodes_end();
 
   bool MadeChange = false;
   while (Position != CurDAG->allnodes_begin()) {
     SDNode *N = &*--Position;
     if (N->use_empty())
       continue;
 
     SDValue Res;
void PPCDAGToDAGISel::PeepholePPC64ZExt() {
   // def : Pat<(i64 (zext i32:$in)),
   //           (RLDICL (INSERT_SUBREG (i64 (IMPLICIT_DEF)), $in, sub_32),
   //                   0, 32)>;
   // There are several 32-bit shift/rotate instructions, however, that will
   // clear the higher-order bits of their output, rendering the RLDICL
   // unnecessary. When that happens, we remove it here, and redefine the
   // relevant 32-bit operation to be a 64-bit operation.
 
-  SelectionDAG::allnodes_iterator Position(CurDAG->getRoot().getNode());
-  ++Position;
+  SelectionDAG::allnodes_iterator Position = CurDAG->allnodes_end();
 
   bool MadeChange = false;
   while (Position != CurDAG->allnodes_begin()) {
     SDNode *N = &*--Position;
     // Skip dead nodes and any non-machine opcodes.
     if (N->use_empty() || !N->isMachineOpcode())
       continue;
 
if (MadeChange)
     CurDAG->RemoveDeadNodes();
 }
 
 void PPCDAGToDAGISel::PeepholePPC64() {
   // These optimizations are currently supported only for 64-bit SVR4.
   if (PPCSubTarget->isDarwin() || !PPCSubTarget->isPPC64())
     return;
 
-  SelectionDAG::allnodes_iterator Position(CurDAG->getRoot().getNode());
-  ++Position;
+  SelectionDAG::allnodes_iterator Position = CurDAG->allnodes_end();
 
   while (Position != CurDAG->allnodes_begin()) {
     SDNode *N = &*--Position;
     // Skip dead nodes and any non-machine opcodes.
     if (N->use_empty() || !N->isMachineOpcode())
       continue;
 
     unsigned FirstOp;

llvm/trunk/test/CodeGen/PowerPC/ppcf128-endian.ll

entry:
   call void @test(ppc_fp128 0xM3FF00000000000000000000000000000)
   ret void
 }
 ; CHECK: .LCPI[[LC:[0-9]+]]_0:
 ; CHECK: .long   1065353216
 ; CHECK: .LCPI[[LC]]_1:
 ; CHECK: .long   0
 ; CHECK: @caller_const
-; CHECK: addi [[REG0:[0-9]+]], {{[0-9]+}}, .LCPI[[LC]]_0
-; CHECK: addi [[REG1:[0-9]+]], {{[0-9]+}}, .LCPI[[LC]]_1
-; CHECK: lfs 1, 0([[REG0]])
-; CHECK: lfs 2, 0([[REG1]])
+; CHECK: addis [[REG0:[0-9]+]], 2, .LCPI[[LC]]_0@toc@ha
+; CHECK: addis [[REG1:[0-9]+]], 2, .LCPI[[LC]]_1@toc@ha
+; CHECK: lfs 1, .LCPI[[LC]]_0@toc@l([[REG0]])
+; CHECK: lfs 2, .LCPI[[LC]]_1@toc@l([[REG1]])
 ; CHECK: bl test
 
 define ppc_fp128 @result() {
 entry:
   %0 = load ppc_fp128, ppc_fp128* @g, align 16
   ret ppc_fp128 %0
 }
 ; CHECK: @result