[PowerPC] Allow absolute expressions in relocations

[nemanjai]

Summary

The Linux kernel build uses absolute expressions suffixed with @lo/@ha relocations. This currently doesn't work for DS/DQ form instructions and there is no reason for it not to. It also works with GAS.
This patch allows this as long as the value is a multiple of 4/16 for DS/DQ form.

[nickdesaulniers]

Thanks for the patch!

llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp

345–367

These two functions look very similar except for one constant. Consider calling a new helper that shares an implementation but accepts the constant as a parameter.

llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCExpr.cpp

114

Using Fixup->getTargetKind() should avoid the need for the casts.

llvm/test/MC/PowerPC/ppc64-abs-reloc.s

4–8

I'm guessing these directives and the cfi directives below are unnecessary to the functionality of this test? I'd be curious if this test case could be pared down further?

[nemanjai]

llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp

345–367

Sure, but I don't really think there's a way to pass parameters to it from the .td description (see lines 1018 and 1032 of PPCInstrInfo.td). I will turn them into wrappers for a function that does the computation.

llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCExpr.cpp

114

Sounds good.

llvm/test/MC/PowerPC/ppc64-abs-reloc.s

4–8

I just left what I got from the back end when compiling. I agree that they are not needed and will remove them.

[nemanjai]

Fold the logic for immediate operands into one function. Simplify the test case and remove some unnecessary casts.

[nickdesaulniers]

llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp

693

the ternary checking Signed will always be true in this branch of the if (Signed) one line above it.

696–697

and this ternary will always be false.

698

should the unsigned branch be calling getImmU16Context instead of getImmS16Context?

[nemanjai]

Hmm... for whatever reason, I forgot that these checks are in signed/unsigned blocks already. Update coming up.

[nemanjai]

Fix the queries for the signed/unsigned cases.

[shchenz]

Maybe I am misunderstanding the relocation fixups and DQ form instructions, if so, please ignore my comments. : )

llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp

47

For DQ form the adjusted value should be Value & 0xfff0? The last 4 bits are 0?

llvm/lib/Target/PowerPC/MCTargetDesc/PPCFixupKinds.h

54

For DQ form, the fixup(DQ field?) should be 12 bits and 4 implied zero bits?

[nemanjai]

In D115419#3247286, @shchenz wrote:

Maybe I am misunderstanding the relocation fixups and DQ form instructions, if so, please ignore my comments. : )

For details of how this stuff works (i.e. constraints on relocations) please see section 3.5.1 Relocation Fields of the ELF V2 ABI Specification.

llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp

47

No, these two are the exact same thing, we just need to differentiate them by name. This is only to differentiate the two when we need to compute and validate the relocations (see the code in PPCMCExpr.cpp).

llvm/lib/Target/PowerPC/MCTargetDesc/PPCFixupKinds.h

54

Oh, for the fixup, it should just be 2 bits (i.e. this is the exact same thing as the ds fixup).

[nickdesaulniers]

Thanks @nemanjai . Consider cleaning up some whitespace nits in the newly added test before comitting.

llvm/test/MC/PowerPC/ppc64-abs-reloc.s

3

remove extra whitespace between -D and -r

4–16

The indentation here looks like mixed usage of spaces and tabs. Let's pick one, and be consistent.

llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp

bool isU7ImmX4() const { return Kind == Immediate &&
                                   (getImm() & 3) == 0; }
   bool isU8Imm() const { return Kind == Immediate && isUInt<8>(getImm()); }
   bool isU8ImmX8() const { return Kind == Immediate &&
                                   isUInt<8>(getImm()) &&
                                   (getImm() & 7) == 0; }
 
   bool isU10Imm() const { return Kind == Immediate && isUInt<10>(getImm()); }
   bool isU12Imm() const { return Kind == Immediate && isUInt<12>(getImm()); }
-  bool isU16Imm() const {
-    switch (Kind) {
-      case Expression:
-        return true;
-      case Immediate:
-      case ContextImmediate:
-        return isUInt<16>(getImmU16Context());
-      default:
-        return false;
-    }
-  }
-  bool isS16Imm() const {
-    switch (Kind) {
-      case Expression:
-        return true;
-      case Immediate:
-      case ContextImmediate:
-        return isInt<16>(getImmS16Context());
-      default:
-        return false;
-    }
-  }
-  bool isS16ImmX4() const { return Kind == Expression ||
-                                   (Kind == Immediate && isInt<16>(getImm()) &&
-                                    (getImm() & 3) == 0); }
+  bool isU16Imm() const { return isExtImm<16>(/*Signed*/ false, 1); }
+  bool isS16Imm() const { return isExtImm<16>(/*Signed*/ true, 1); }
+  bool isS16ImmX4() const { return isExtImm<16>(/*Signed*/ true, 4); }
+  bool isS16ImmX16() const { return isExtImm<16>(/*Signed*/ true, 16); }
+  bool isS17Imm() const { return isExtImm<17>(/*Signed*/ true, 1); }
 
   bool isHashImmX8() const {
     // The Hash Imm form is used for instructions that check or store a hash.
     // These instructions have a small immediate range that spans between
     // -8 and -512.
     return (Kind == Immediate && getImm() <= -8 && getImm() >= -512 &&
             (getImm() & 7) == 0);
   }
 
-  bool isS16ImmX16() const { return Kind == Expression ||
-                                    (Kind == Immediate && isInt<16>(getImm()) &&
-                                     (getImm() & 15) == 0); }
   bool isS34ImmX16() const {
     return Kind == Expression ||
            (Kind == Immediate && isInt<34>(getImm()) && (getImm() & 15) == 0);
   }
   bool isS34Imm() const {
     // Once the PC-Rel ABI is finalized, evaluate whether a 34-bit
     // ContextImmediate is needed.
     return Kind == Expression || (Kind == Immediate && isInt<34>(getImm()));
   }
 
-  bool isS17Imm() const {
-    switch (Kind) {
-      case Expression:
-        return true;
-      case Immediate:
-      case ContextImmediate:
-        return isInt<17>(getImmS16Context());
-      default:
-        return false;
-    }
-  }
   bool isTLSReg() const { return Kind == TLSRegister; }
   bool isDirectBr() const {
     if (Kind == Expression)
       return true;
     if (Kind != Immediate)
       return false;
     // Operand must be 64-bit aligned, signed 27-bit immediate.
     if ((getImm() & 3) != 0)
CreateFromMCExpr(const MCExpr *Val, SMLoc S, SMLoc E, bool IsPPC64) {
     if (const PPCMCExpr *TE = dyn_cast<PPCMCExpr>(Val)) {
       int64_t Res;
       if (TE->evaluateAsConstant(Res))
         return CreateContextImm(Res, S, E, IsPPC64);
     }
 
     return CreateExpr(Val, S, E, IsPPC64);
   }
+
+private:
+  template <unsigned Width>
+  bool isExtImm(bool Signed, unsigned Multiple) const {
+    switch (Kind) {
+    default:
+      return false;
+    case Expression:
+      return true;
+    case Immediate:
+    case ContextImmediate:
+      if (Signed)
+        return isInt<Width>(getImmS16Context()) &&
+               (getImmS16Context() & (Multiple - 1)) == 0;
+      else
+        return isUInt<Width>(getImmU16Context()) &&
+               (getImmU16Context() & (Multiple - 1)) == 0;
+    }
+  }
 };
 
 } // end anonymous namespace.
 
 void PPCOperand::print(raw_ostream &OS) const {
   switch (Kind) {
   case Token:
     OS << "'" << getToken() << "'";

llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp

case PPC::fixup_ppc_brcond14abs:
     return Value & 0xfffc;
   case PPC::fixup_ppc_br24:
   case PPC::fixup_ppc_br24abs:
   case PPC::fixup_ppc_br24_notoc:
     return Value & 0x3fffffc;
   case PPC::fixup_ppc_half16:
     return Value & 0xffff;
   case PPC::fixup_ppc_half16ds:
+  case PPC::fixup_ppc_half16dq:
     return Value & 0xfffc;
   case PPC::fixup_ppc_pcrel34:
   case PPC::fixup_ppc_imm34:
     return Value & 0x3ffffffff;
   }
 }
 
 static unsigned getFixupKindNumBytes(unsigned Kind) {
   switch (Kind) {
   default:
     llvm_unreachable("Unknown fixup kind!");
   case FK_Data_1:
     return 1;
   case FK_Data_2:
   case PPC::fixup_ppc_half16:
   case PPC::fixup_ppc_half16ds:
+  case PPC::fixup_ppc_half16dq:
     return 2;
   case FK_Data_4:
   case PPC::fixup_ppc_brcond14:
   case PPC::fixup_ppc_brcond14abs:
   case PPC::fixup_ppc_br24:
   case PPC::fixup_ppc_br24abs:
   case PPC::fixup_ppc_br24_notoc:
     return 4;

llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp

case PPC::fixup_ppc_half16:
         Type = ELF::R_PPC_REL16_HI;
         break;
       case MCSymbolRefExpr::VK_PPC_HA:
         Type = ELF::R_PPC_REL16_HA;
         break;
       }
       break;
     case PPC::fixup_ppc_half16ds:
+    case PPC::fixup_ppc_half16dq:
       Target.print(errs());
       errs() << '\n';
       report_fatal_error("Invalid PC-relative half16ds relocation");
     case PPC::fixup_ppc_pcrel34:
       switch (Modifier) {
       default:
         llvm_unreachable("Unsupported Modifier for fixup_ppc_pcrel34");
       case MCSymbolRefExpr::VK_PCREL:
case PPC::fixup_ppc_half16:
         Type = ELF::R_PPC64_GOT_DTPREL16_HI;
         break;
       case MCSymbolRefExpr::VK_PPC_GOT_DTPREL_HA:
         Type = ELF::R_PPC64_GOT_DTPREL16_HA;
         break;
       }
       break;
     case PPC::fixup_ppc_half16ds:
+    case PPC::fixup_ppc_half16dq:
       switch (Modifier) {
       default: llvm_unreachable("Unsupported Modifier");
       case MCSymbolRefExpr::VK_None:
         Type = ELF::R_PPC64_ADDR16_DS;
         break;
       case MCSymbolRefExpr::VK_PPC_LO:
         Type = ELF::R_PPC64_ADDR16_LO_DS;
         break;

llvm/lib/Target/PowerPC/MCTargetDesc/PPCFixupKinds.h

enum Fixups {
   // A 34-bit fixup corresponding to Non-PC-relative paddi.
   fixup_ppc_imm34,
 
   /// Not a true fixup, but ties a symbol to a call to __tls_get_addr for the
   /// TLS general and local dynamic models, or inserts the thread-pointer
   /// register number.
   fixup_ppc_nofixup,
 
+  /// A 16-bit fixup corresponding to lo16(_foo) with implied 3 zero bits for
+  /// instrs like 'lxv'. Produces the same relocation as fixup_ppc_half16ds.
+  fixup_ppc_half16dq,
+
   // Marker
   LastTargetFixupKind,
   NumTargetFixupKinds = LastTargetFixupKind - FirstTargetFixupKind
 };
 }
 }
 
 #endif

llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp

unsigned PPCMCCodeEmitter::getMemRIX16Encoding(const MCInst &MI, unsigned OpNo,
   const MCOperand &MO = MI.getOperand(OpNo);
   if (MO.isImm()) {
     assert(!(MO.getImm() % 16) &&
            "Expecting an immediate that is a multiple of 16");
     return ((getMachineOpValue(MI, MO, Fixups, STI) >> 4) & 0xFFF) | RegBits;
   }
 
   // Otherwise add a fixup for the displacement field.
-  Fixups.push_back(MCFixup::create(IsLittleEndian? 0 : 2, MO.getExpr(),
-                                   (MCFixupKind)PPC::fixup_ppc_half16ds));
+  Fixups.push_back(MCFixup::create(IsLittleEndian ? 0 : 2, MO.getExpr(),
+                                   (MCFixupKind)PPC::fixup_ppc_half16dq));
   return RegBits;
 }
 
 unsigned
 PPCMCCodeEmitter::getMemRIHashEncoding(const MCInst &MI, unsigned OpNo,
                                        SmallVectorImpl<MCFixup> &Fixups,
                                        const MCSubtargetInfo &STI) const {
   // Encode (imm, reg) for the hash load/store to stack for the ROP Protection

llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCExpr.cpp

PPCMCExpr::evaluateAsRelocatableImpl(MCValue &Res,
                                      const MCFixup *Fixup) const {
   MCValue Value;
 
   if (!getSubExpr()->evaluateAsRelocatable(Value, Layout, Fixup))
     return false;
 
   if (Value.isAbsolute()) {
     int64_t Result = evaluateAsInt64(Value.getConstant());
-    if ((Fixup == nullptr || (unsigned)Fixup->getKind() != PPC::fixup_ppc_half16) &&
-        (Result >= 0x8000))
+    bool IsHalf16 = Fixup && Fixup->getTargetKind() == PPC::fixup_ppc_half16;
+    bool IsHalf16DS =
+        Fixup && Fixup->getTargetKind() == PPC::fixup_ppc_half16ds;
+    bool IsHalf16DQ =
+        Fixup && Fixup->getTargetKind() == PPC::fixup_ppc_half16dq;
+    bool IsHalf = IsHalf16 || IsHalf16DS || IsHalf16DQ;
+
+    if (!IsHalf && Result >= 0x8000)
+      return false;
+    if ((IsHalf16DS && (Result & 0x3)) || (IsHalf16DQ && (Result & 0xf)))
       return false;
+
     Res = MCValue::get(Result);
   } else {
     if (!Layout)
       return false;
 
     MCContext &Context = Layout->getAssembler().getContext();
     const MCSymbolRefExpr *Sym = Value.getSymA();
     MCSymbolRefExpr::VariantKind Modifier = Sym->getKind();

llvm/lib/Target/PowerPC/PPCInstrInfo.td

  let Name = "DispRI"; let PredicateMethod = "isS16Imm";
  let RenderMethod = "addS16ImmOperands";
 }
 def dispRI : Operand<iPTR> {
   let ParserMatchClass = PPCDispRIOperand;
 }
 def PPCDispRIXOperand : AsmOperandClass {
  let Name = "DispRIX"; let PredicateMethod = "isS16ImmX4";
- let RenderMethod = "addImmOperands";
+ let RenderMethod = "addS16ImmOperands";
 }
 def dispRIX : Operand<iPTR> {
   let ParserMatchClass = PPCDispRIXOperand;
 }
 def PPCDispRIHashOperand : AsmOperandClass {
   let Name = "DispRIHash"; let PredicateMethod = "isHashImmX8";
   let RenderMethod = "addImmOperands";
 }
 def dispRIHash : Operand<iPTR> {
   let ParserMatchClass = PPCDispRIHashOperand;
 }
 def PPCDispRIX16Operand : AsmOperandClass {
  let Name = "DispRIX16"; let PredicateMethod = "isS16ImmX16";
- let RenderMethod = "addImmOperands";
+ let RenderMethod = "addS16ImmOperands";
 }
 def dispRIX16 : Operand<iPTR> {
   let ParserMatchClass = PPCDispRIX16Operand;
 }
 def PPCDispSPE8Operand : AsmOperandClass {
  let Name = "DispSPE8"; let PredicateMethod = "isU8ImmX8";
  let RenderMethod = "addImmOperands";
 }

llvm/test/MC/PowerPC/ppc64-abs-reloc.s

+# RUN: llvm-mc -triple powerpc64le-unknown-linux-gnu %s -filetype=obj -o - | \
+# RUN:    llvm-objdump -D -r - | FileCheck %s
+  .text
+test:                                   # @test
+        add 5, 3, 4
+        extsw 3, 5
+        .space 32776
+lab2:
+        lxv 5, (lab2-test)@l(4)
+        ld 5, (lab2-test)@l(4)
+        lwz 5, (lab2-test)@l(4)
+        lxv 5, 8389632@l(4)
+        ld 5, 8389632@l(4)
+        lwz 5, 8389632@l(4)
+        blr
+
+# CHECK: lxv 5, -32752(4)
+# CHECK: ld 5, -32752(4)
+# CHECK: lwz 5, -32752(4)
+# CHECK: lxv 5, 1024(4)
+# CHECK: ld 5, 1024(4)
+# CHECK: lwz 5, 1024(4)