[PATCH] D13649: [mips] Clang ll/sc illegal instruction on mips64r2 with -O0

[Daniel Sanders via llvm-commits]

dsanders requested changes to this revision.
dsanders added a comment.
This revision now requires changes to proceed.

The latest patch only deals with part of the problem. I've commented on the first few pointer/integer inconsistencies but there are more.


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1413
@@ -1412,2 +1412,3 @@
   MachineRegisterInfo &RegInfo = MF->getRegInfo();
-  const TargetRegisterClass *RC = getRegClassFor(MVT::i32);
+  bool IsABI64 = Subtarget.isABI_N64();
+  const TargetRegisterClass *RC = 
----------------
Nit: For consistency please use 'IsABIN64'. It will be confusing to use '64' and 'N64' interchangably. Or preferably, refer to the pointer size instead to make it easier to keep track of the difference between integer and pointer sizes.

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1424-1440
@@ -1421,19 +1423,19 @@
 
   unsigned AlignedAddr = RegInfo.createVirtualRegister(RC);
   unsigned ShiftAmt = RegInfo.createVirtualRegister(RC);
   unsigned Mask = RegInfo.createVirtualRegister(RC);
   unsigned Mask2 = RegInfo.createVirtualRegister(RC);
   unsigned ShiftedCmpVal = RegInfo.createVirtualRegister(RC);
   unsigned OldVal = RegInfo.createVirtualRegister(RC);
   unsigned MaskedOldVal0 = RegInfo.createVirtualRegister(RC);
   unsigned ShiftedNewVal = RegInfo.createVirtualRegister(RC);
   unsigned MaskLSB2 = RegInfo.createVirtualRegister(RC);
   unsigned PtrLSB2 = RegInfo.createVirtualRegister(RC);
   unsigned MaskUpper = RegInfo.createVirtualRegister(RC);
   unsigned MaskedCmpVal = RegInfo.createVirtualRegister(RC);
   unsigned MaskedNewVal = RegInfo.createVirtualRegister(RC);
   unsigned MaskedOldVal1 = RegInfo.createVirtualRegister(RC);
   unsigned StoreVal = RegInfo.createVirtualRegister(RC);
   unsigned SrlRes = RegInfo.createVirtualRegister(RC);
   unsigned Success = RegInfo.createVirtualRegister(RC);
 
----------------
Some of these should pick i64 for 64-bit integers (N32/N64), others should pick i64 for 64-bit pointers (N64 only).

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1480
@@ -1477,2 +1479,3 @@
   int64_t MaskImm = (Size == 1) ? 255 : 65535;
   BuildMI(BB, DL, TII->get(Mips::ADDiu), MaskLSB2)
+    .addReg(ABI.GetZeroReg()).addImm(-4);
----------------
This should pick between ADDiu and DADDiu depending on whether we have 32/64 bit pointers (at first it looks like an integer but it will be combined with a pointer in the next insn). You'll need to switch to GetNullPtr() too.

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1487
@@ -1483,3 +1486,3 @@
   if (Subtarget.isLittle()) {
     BuildMI(BB, DL, TII->get(Mips::SLL), ShiftAmt).addReg(PtrLSB2).addImm(3);
   } else {
----------------
SLL/DSLL depending on pointer size

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1492
@@ -1488,3 +1491,3 @@
       .addReg(PtrLSB2).addImm((Size == 1) ? 3 : 2);
     BuildMI(BB, DL, TII->get(Mips::SLL), ShiftAmt).addReg(Off).addImm(3);
   }
----------------
SLL/DSLL depending on pointer size

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1494-1495
@@ -1490,4 +1493,4 @@
   }
-  BuildMI(BB, DL, TII->get(Mips::ORi), MaskUpper)
-    .addReg(Mips::ZERO).addImm(MaskImm);
+  BuildMI(BB, DL, TII->get(IsABI64 ? Mips::ORi64 : Mips::ORi), MaskUpper)
+    .addReg(ABI.GetZeroReg()).addImm(MaskImm);
   BuildMI(BB, DL, TII->get(Mips::SLLV), Mask)
----------------
IsABI64 and GetZeroReg() do not agree with eachother.

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1496
@@ -1493,2 +1495,3 @@
+    .addReg(ABI.GetZeroReg()).addImm(MaskImm);
   BuildMI(BB, DL, TII->get(Mips::SLLV), Mask)
     .addReg(MaskUpper).addReg(ShiftAmt);
----------------
SLL/DSLLV based on pointer size (I think)

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1498-1499
@@ -1494,3 +1497,4 @@
     .addReg(MaskUpper).addReg(ShiftAmt);
-  BuildMI(BB, DL, TII->get(Mips::NOR), Mask2).addReg(Mips::ZERO).addReg(Mask);
-  BuildMI(BB, DL, TII->get(Mips::ANDi), MaskedCmpVal)
+  BuildMI(BB, DL, TII->get(IsABI64 ? Mips::NOR64 : Mips::NOR), Mask2)
+    .addReg(ABI.GetZeroReg()).addReg(Mask);
+  BuildMI(BB, DL, TII->get(IsABI64 ? Mips::ANDi64 : Mips::ANDi), MaskedCmpVal)
----------------
IsABI64 and GetZeroReg() do not agree with eachother.

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1502
@@ -1497,3 +1501,3 @@
     .addReg(CmpVal).addImm(MaskImm);
   BuildMI(BB, DL, TII->get(Mips::SLLV), ShiftedCmpVal)
     .addReg(MaskedCmpVal).addReg(ShiftAmt);
----------------
SLLV/DSLLV. It's not immediately obvious whether this is integer or pointer

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1506
@@ -1501,3 +1505,3 @@
     .addReg(NewVal).addImm(MaskImm);
   BuildMI(BB, DL, TII->get(Mips::SLLV), ShiftedNewVal)
     .addReg(MaskedNewVal).addReg(ShiftAmt);
----------------
SLLV/DSLLV. It's not immediately obvious whether this is integer or pointer


http://reviews.llvm.org/D13649