[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

I would just use a single new line here.

lib/Target/ARM/ARMInstrInfo.td

4813

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

Same comment about Sched<[WriteALU]>

[samparker]

lib/Target/ARM/ARM.td

89

cheers.

lib/Target/ARM/ARMInstrInfo.td

4813

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

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

Please remove SDCOMP-27699.

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

1

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

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

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

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

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

1

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

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

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.

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">;

                                                        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",

lib/Target/ARM/ARMInstrInfo.td

                                  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",

 // ISB has only full system option
 def ISB : AInoP<(outs), (ins instsyncb_opt:$opt), MiscFrm, NoItinerary,
                 "isb", "\t$opt", [(int_arm_isb (i32 imm0_15:$opt))]>,
                 Requires<[IsARM, HasDB]> {
   bits<4> opt;
   let Inst{31-4} = 0xf57ff06;
   let Inst{3-0} = opt;
 }
+
+def DFB : AInoP<(outs), (ins), MiscFrm, NoItinerary,
+                "dfb", "", []>,
+                Requires<[IsARM, HasDFB]> , Sched<[WriteALU]> {
+                let Inst{31-0} = 0xf57ff04c;
+}
 }
 
 let usesCustomInserter = 1, Defs = [CPSR] in {
 
 // Pseudo instruction that combines movs + predicated rsbmi
 // to implement integer ABS
   def ABS : ARMPseudoInst<(outs GPR:$dst), (ins GPR:$src), 8, NoItinerary, []>;
 }

lib/Target/ARM/ARMInstrThumb2.td

 
 def t2ISB : T2I<(outs), (ins instsyncb_opt:$opt), NoItinerary,
                 "isb", "\t$opt", [(int_arm_isb (i32 imm0_15:$opt))]>,
                 Requires<[IsThumb, HasDB]> {
   bits<4> opt;
   let Inst{31-4} = 0xf3bf8f6;
   let Inst{3-0} = opt;
 }
+
+def t2DFB : T2I<(outs), (ins), NoItinerary,
+                "dfb", "", []>,
+                Requires<[IsThumb, HasDFB]>, Sched<[WriteALU]> {
+                let Inst{31-0} = 0xf3bf8f4c;
+}
 }
 
 class T2I_ldrex<bits<4> opcod, dag oops, dag iops, AddrMode am, int sz,
                 InstrItinClass itin, string opc, string asm, string cstr,
                 list<dag> pattern, bits<4> rt2 = 0b1111>
   : Thumb2I<oops, iops, am, sz, itin, opc, asm, cstr, pattern> {
   let Inst{31-27} = 0b11101;
   let Inst{26-20} = 0b0001101;

   let Inst{7-0} = 0b11101111;
 }
 let hasExtraDefRegAllocReq = 1 in
 def t2LDAEXD : T2I_ldrex<0b1111, (outs rGPR:$Rt, rGPR:$Rt2),
                          (ins addr_offset_none:$addr),
                          AddrModeNone, 4, NoItinerary,
                          "ldaexd", "\t$Rt, $Rt2, $addr", "",
                          [], {?, ?, ?, ?}>, Requires<[IsThumb,
                          HasAcquireRelease, HasV7Clrex, IsNotMClass]> {

lib/Target/ARM/ARMSubtarget.h

 
   /// 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

 
   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; }

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

       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;
 }

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

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]

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

+# SDCOMP-27699: DFB on ARMv8-R
+# 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]

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

+# SDCOMP-27699: DFB on ARMv8-R
+# 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]