[ARM] Armv8-R DFB instruction

[samparker]

Summary

Implement MC support for the Armv8-R 'Data Full Barrier' instruction.

[fhahn]

lib/Target/ARM/ARM.td

89 ↗

(On Diff #127670)

I would just use a single new line here.

lib/Target/ARM/ARMInstrInfo.td

4813 ↗

(On Diff #127670)

Are you sure about Sched<[WriteALU]>? It's an alias to the DSB instruction , which does not have a similar Sched

lib/Target/ARM/ARMInstrThumb2.td

3197 ↗

(On Diff #127670)

Same comment about Sched<[WriteALU]>

[samparker]

lib/Target/ARM/ARM.td

89 ↗

(On Diff #127670)

cheers.

lib/Target/ARM/ARMInstrInfo.td

4813 ↗

(On Diff #127670)

I'm not, but couldn't find where/how DSB got its scheduling info. Do you know where its defined? I should probably also add an InstAlias to DSB...

[fhahn]

This is the only new instruction added in Armv8-r, right? I assume this is the reason you added a very specific target feature for it (rather than something like armv8-r)? I think this also the reason you had to add 4 separate tiny test files? This is slightly unfortunate, but matches with the way things are done currently.

lib/Target/ARM/ARMInstrInfo.td

4813 ↗

(On Diff #127670)

The instruction is marked as having (unmodelled) side effects. That should tell the scheduler to not move instructions past this instruction, so I think there is no need to add scheduling info.

test/MC/Disassembler/ARM/dfb-arm.txt

1 ↗

(On Diff #127670)

Please remove SDCOMP-27699.

test/MC/Disassembler/ARM/dfb-thumb.txt

1 ↗

(On Diff #127670)

Please remove SDCOMP-27699.

[fhahn]

Also, for reference, https://developer.arm.com/products/architecture/r-profile/docs/ddi0568/latest/arm-architecture-reference-manual-supplement-armv8-for-the-armv8-r-aarch32-architecture-profile contains info about DFB.

[javed.absar]

lib/Target/ARM/ARMInstrInfo.td

4813 ↗

(On Diff #127670)

As this is memory relate WriteLd may be a better option (and probably more realistic in terms of timing generally).

[sdesmalen]

lib/Target/ARM/ARMInstrInfo.td

4813 ↗

(On Diff #127670)

Having an InstAlias to DSB instead of a new instruction allows you to reuse the original instruction definition (and not have to worry about scheduling models).
TableGen usually errors when it finds a collision in instruction encodings, so I'm surprised it doesn't for this case (i.e. this instruction would collide with DSB instruction with immediate #0xc).

[samparker]

lib/Target/ARM/ARMInstrInfo.td

4813 ↗

(On Diff #127670)

Yes, cheers, this is what I'm doing instead.

test/MC/Disassembler/ARM/dfb-arm.txt

1 ↗

(On Diff #127670)

Thanks!

[samparker]

@sdesmalen The architecture manual states that DFB is always the preferred disassembly output. So do you know if its possible to do an InstAlias which would produce the 'dfb' string when a dsb #12 is disassembled? Seems cleaner to me to remove the instruction definition, but I'm not sure thats its trivial.

[sdesmalen]

In D41430#960723, @samparker wrote:

@sdesmalen The architecture manual states that DFB is always the preferred disassembly output. So do you know if its possible to do an InstAlias which would produce the 'dfb' string when a dsb #12 is disassembled? Seems cleaner to me to remove the instruction definition, but I'm not sure thats its trivial.

You can set the EmitPriority to 1 (or higher) so that it is always printed when 'dsb #12' is disassembled. (See the comments in include/llvm/Target/Target.td for EmitPriority).

[samparker]

great, i'll try that. thanks

[samparker]

Removed whitespace changes and SDCOMP messages and also removed the instruction definitions in favour of using aliases.

[sdesmalen]

lib/Target/ARM/ARMInstrThumb2.td

4512 ↗

(On Diff #127696)

Thanks for making this change. Also, if you change the last parameter of InstAlias to '1', you don't need to set EmitPriority=1.
i.e.

IntAlias<"dfb${p}", (t2DSB 0xc, pred:$p), 1>, Requires<[HasDFB]>;

[samparker]

lib/Target/ARM/ARMInstrThumb2.td

4512 ↗

(On Diff #127696)

Ah! Will do

[samparker]

Passing '1' to set EmitPriority.

[sdesmalen]

In D41430#960837, @samparker wrote:

Passing '1' to set EmitPriority.

Thanks, it looks much better now.

[fhahn]

LGTM thanks Sam

[fhahn]

nit: Maybe use [ARM] Add Armv8-R DFB instruction. as a title?

[samparker]

Thanks for the review. Would either of you mind committing this for me? My computer has just been taken for a rebuild.

[fhahn]

In D41430#960936, @samparker wrote:

Thanks for the review. Would either of you mind committing this for me? My computer has just been taken for a rebuild.

I could commit it for your, but I think it's fine to commit this in a couple of days.

[samparker]

Ok, no worries then.

llvm/trunk/lib/Target/ARM/ARM.td

 
 // Atomic Support
 def FeatureDB             : SubtargetFeature<"db", "HasDataBarrier", "true",
                                              "Has data barrier (dmb/dsb) instructions">;
 
 def FeatureV7Clrex        : SubtargetFeature<"v7clrex", "HasV7Clrex", "true",
                                              "Has v7 clrex instruction">;
 
+def FeatureDFB  : SubtargetFeature<"dfb", "HasFullDataBarrier", "true",
+                                   "Has full data barrier (dfb) instruction">;
+
 def FeatureAcquireRelease : SubtargetFeature<"acquire-release",
                                              "HasAcquireRelease", "true",
                                              "Has v8 acquire/release (lda/ldaex "
                                              " etc) instructions">;
 
 
 def FeatureSlowFPBrcc     : SubtargetFeature<"slow-fp-brcc", "SlowFPBrcc", "true",
                                              "FP compare + branch is slow">;
def ARMv83a   : Architecture<"armv8.3-a", "ARMv83a",  [HasV8_3aOps,
                                                        FeatureVirtualization,
                                                        FeatureCrypto,
                                                        FeatureCRC,
                                                        FeatureRAS]>;
 
 def ARMv8r    : Architecture<"armv8-r",   "ARMv8r",   [HasV8Ops,
                                                        FeatureRClass,
                                                        FeatureDB,
+                                                       FeatureDFB,
                                                        FeatureDSP,
                                                        FeatureCRC,
                                                        FeatureMP,
                                                        FeatureVirtualization,
                                                        FeatureFPARMv8,
                                                        FeatureNEON]>;
 
 def ARMv8mBaseline : Architecture<"armv8-m.base", "ARMv8mBaseline",

llvm/trunk/lib/Target/ARM/ARMInstrInfo.td

def HasDivideInThumb : Predicate<"Subtarget->hasDivideInThumbMode()">,
                                  AssemblerPredicate<"FeatureHWDivThumb", "divide in THUMB">;
 def HasDivideInARM   : Predicate<"Subtarget->hasDivideInARMMode()">,
                                  AssemblerPredicate<"FeatureHWDivARM", "divide in ARM">;
 def HasDSP           : Predicate<"Subtarget->hasDSP()">,
                                  AssemblerPredicate<"FeatureDSP", "dsp">;
 def HasDB            : Predicate<"Subtarget->hasDataBarrier()">,
                                  AssemblerPredicate<"FeatureDB",
                                                     "data-barriers">;
+def HasDFB           : Predicate<"Subtarget->hasFullDataBarrier()">,
+                                 AssemblerPredicate<"FeatureDFB",
+                                                    "full-data-barrier">;
 def HasV7Clrex  : Predicate<"Subtarget->hasV7Clrex()">,
                             AssemblerPredicate<"FeatureV7Clrex",
                                                "v7 clrex">;
 def HasAcquireRelease : Predicate<"Subtarget->hasAcquireRelease()">,
                                   AssemblerPredicate<"FeatureAcquireRelease",
                                                      "acquire/release">;
 def HasMP            : Predicate<"Subtarget->hasMPExtension()">,
                                  AssemblerPredicate<"FeatureMP",

 //===----------------------------------------------------------------------===//
 // Assembler aliases
 //
 
 // Memory barriers
 def : InstAlias<"dmb", (DMB 0xf), 0>, Requires<[IsARM, HasDB]>;
 def : InstAlias<"dsb", (DSB 0xf), 0>, Requires<[IsARM, HasDB]>;
 def : InstAlias<"isb", (ISB 0xf), 0>, Requires<[IsARM, HasDB]>;
+// Armv8-R 'Data Full Barrier'
+def : InstAlias<"dfb", (DSB 0xc), 1>, Requires<[IsARM, HasDFB]>;
 
 // System instructions
 def : MnemonicAlias<"swi", "svc">;
 
 // Load / Store Multiple
 def : MnemonicAlias<"ldmfd", "ldm">;
 def : MnemonicAlias<"ldmia", "ldm">;
 def : MnemonicAlias<"ldmea", "ldmdb">;

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

def : t2InstAlias<"teq${p} $Rn, $Rm",
                   (t2TEQrr GPRnopc:$Rn, rGPR:$Rm, pred:$p)>;
 def : t2InstAlias<"tst${p} $Rn, $Rm",
                   (t2TSTrr GPRnopc:$Rn, rGPR:$Rm, pred:$p)>;
 
 // Memory barriers
 def : InstAlias<"dmb${p}", (t2DMB 0xf, pred:$p), 0>, Requires<[HasDB]>;
 def : InstAlias<"dsb${p}", (t2DSB 0xf, pred:$p), 0>, Requires<[HasDB]>;
 def : InstAlias<"isb${p}", (t2ISB 0xf, pred:$p), 0>, Requires<[HasDB]>;
+// Armv8-R 'Data Full Barrier'
+def : InstAlias<"dfb${p}", (t2DSB 0xc, pred:$p), 1>, Requires<[HasDFB]>;
 
 // Alias for LDR, LDRB, LDRH, LDRSB, and LDRSH without the ".w" optional
 // width specifier.
 def : t2InstAlias<"ldr${p} $Rt, $addr",
                   (t2LDRi12 GPR:$Rt, t2addrmode_imm12:$addr, pred:$p)>;
 def : t2InstAlias<"ldrb${p} $Rt, $addr",
                   (t2LDRBi12 rGPR:$Rt, t2addrmode_imm12:$addr, pred:$p)>;
 def : t2InstAlias<"ldrh${p} $Rt, $addr",

llvm/trunk/lib/Target/ARM/ARMSubtarget.h

protected:
 
   /// HasHardwareDivideInARM - True if subtarget supports [su]div in ARM mode
   bool HasHardwareDivideInARM = false;
 
   /// HasDataBarrier - True if the subtarget supports DMB / DSB data barrier
   /// instructions.
   bool HasDataBarrier = false;
 
+  /// HasFullDataBarrier - True if the subtarget supports DFB data barrier
+  /// instruction.
+  bool HasFullDataBarrier = false;
+
   /// HasV7Clrex - True if the subtarget supports CLREX instructions
   bool HasV7Clrex = false;
 
   /// HasAcquireRelease - True if the subtarget supports v8 atomics (LDA/LDAEX etc)
   /// instructions
   bool HasAcquireRelease = false;
 
   /// Pref32BitThumb - If true, codegen would prefer 32-bit Thumb instructions
public:
 
   bool useNEONForSinglePrecisionFP() const {
     return hasNEON() && UseNEONForSinglePrecisionFP;
   }
 
   bool hasDivideInThumbMode() const { return HasHardwareDivideInThumb; }
   bool hasDivideInARMMode() const { return HasHardwareDivideInARM; }
   bool hasDataBarrier() const { return HasDataBarrier; }
+  bool hasFullDataBarrier() const { return HasFullDataBarrier; }
   bool hasV7Clrex() const { return HasV7Clrex; }
   bool hasAcquireRelease() const { return HasAcquireRelease; }
 
   bool hasAnyDataBarrier() const {
     return HasDataBarrier || (hasV6Ops() && !isThumb());
   }
 
   bool useMulOps() const { return UseMulOps; }

llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

if (Mnemonic == "bkpt" || Mnemonic == "cbnz" || Mnemonic == "setend" ||
       Mnemonic == "vcmla" || Mnemonic == "vcadd") {
     // These mnemonics are never predicable
     CanAcceptPredicationCode = false;
   } else if (!isThumb()) {
     // Some instructions are only predicable in Thumb mode
     CanAcceptPredicationCode =
         Mnemonic != "cdp2" && Mnemonic != "clrex" && Mnemonic != "mcr2" &&
         Mnemonic != "mcrr2" && Mnemonic != "mrc2" && Mnemonic != "mrrc2" &&
-        Mnemonic != "dmb" && Mnemonic != "dsb" && Mnemonic != "isb" &&
-        Mnemonic != "pld" && Mnemonic != "pli" && Mnemonic != "pldw" &&
-        Mnemonic != "ldc2" && Mnemonic != "ldc2l" && Mnemonic != "stc2" &&
-        Mnemonic != "stc2l" && !Mnemonic.startswith("rfe") &&
-        !Mnemonic.startswith("srs");
+        Mnemonic != "dmb" && Mnemonic != "dfb" && Mnemonic != "dsb" &&
+        Mnemonic != "isb" && Mnemonic != "pld" && Mnemonic != "pli" &&
+        Mnemonic != "pldw" && Mnemonic != "ldc2" && Mnemonic != "ldc2l" &&
+        Mnemonic != "stc2" && Mnemonic != "stc2l" &&
+        !Mnemonic.startswith("rfe") && !Mnemonic.startswith("srs");
   } else if (isThumbOne()) {
     if (hasV6MOps())
       CanAcceptPredicationCode = Mnemonic != "movs";
     else
       CanAcceptPredicationCode = Mnemonic != "nop" && Mnemonic != "movs";
   } else
     CanAcceptPredicationCode = true;
 }

llvm/trunk/test/MC/ARM/dfb-neg.s

+@ RUN: not llvm-mc -triple armv8-none-eabi -mcpu=cortex-r52 -mattr=-dfb -show-encoding < %s 2>&1 | FileCheck %s
+@ RUN: not llvm-mc -triple thumbv8-none-eabi -mcpu=cortex-r52 -mattr=-dfb -show-encoding < %s 2>&1 | FileCheck %s
+
+        dfb
+@ CHECK: error: instruction requires: full-data-barrier
+
+        dfb sy
+        dfb #0
+@ CHECK: error: invalid instruction
+@ CHECK: error: invalid instruction

llvm/trunk/test/MC/ARM/dfb.s

+@ RUN: llvm-mc -triple armv8-none-eabi -mcpu=cortex-r52 -show-encoding < %s | FileCheck %s --check-prefix=CHECK-ARM
+@ RUN: llvm-mc -triple thumbv8-none-eabi -mcpu=cortex-r52 -show-encoding < %s | FileCheck %s --check-prefix=CHECK-THUMB
+
+        dfb
+@ CHECK-ARM:   dfb                             @ encoding: [0x4c,0xf0,0x7f,0xf5]
+@ CHECK-THUMB: dfb                             @ encoding: [0xbf,0xf3,0x4c,0x8f]

llvm/trunk/test/MC/Disassembler/ARM/dfb-arm.txt

+# RUN: llvm-mc -disassemble -triple armv8-none-eabi -mcpu=cortex-r52 -show-encoding < %s | FileCheck %s --check-prefix=CHECK-DFB
+# RUN: llvm-mc -disassemble -triple armv8-none-eabi -mcpu=cortex-r52 -mattr=-dfb -show-encoding < %s | FileCheck %s --check-prefix=CHECK-NODFB
+
+# CHECK-DFB:   dfb                             @ encoding: [0x4c,0xf0,0x7f,0xf5]
+# CHECK-NODFB: dsb     #0xc                    @ encoding: [0x4c,0xf0,0x7f,0xf5]
+[0x4c,0xf0,0x7f,0xf5]

llvm/trunk/test/MC/Disassembler/ARM/dfb-thumb.txt

+# RUN: llvm-mc -disassemble -triple thumbv8-none-eabi -mcpu=cortex-r52 -show-encoding < %s | FileCheck %s --check-prefix=CHECK-DFB
+# RUN: llvm-mc -disassemble -triple thumbv8-none-eabi -mcpu=cortex-r52 -mattr=-dfb -show-encoding < %s | FileCheck %s --check-prefix=CHECK-NODFB
+
+# CHECK-DFB:   dfb                             @ encoding: [0xbf,0xf3,0x4c,0x8f]
+# CHECK-NODFB: dsb     #0xc                    @ encoding: [0xbf,0xf3,0x4c,0x8f]
+[0xbf,0xf3,0x4c,0x8f]