[X86] Avoid SFB - Ignore meta instructions

[cdawson]

Summary

This patch generalizes the fix in D61680 to ignore all meta instructions, not just debug info.

[aprantl]

llvm/test/CodeGen/X86/avoid-sfb-ignore-meta.mir

46

Please remove the string-attributes, they are probably not necessary and hard to maintain.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

 /// const_instr_iterator} and the respective reverse iterators.
 template<class IterT>
 inline IterT skipDebugInstructionsBackward(IterT It, IterT Begin) {
   while (It != Begin && It->isDebugInstr())
     It--;
   return It;
 }
 
+/// Increment \p It until it points to a non-meta instruction or to \p End
+/// and return the resulting iterator. This function should only be used
+/// MachineBasicBlock::{iterator, const_iterator, instr_iterator,
+/// const_instr_iterator} and the respective reverse iterators.
+template <typename IterT>
+inline IterT skipMetaInstructionsForward(IterT It, IterT End) {
+  while (It != End && It->isMetaInstruction())
+    It++;
+  return It;
+}
+
+/// Decrement \p It until it points to a non-meta instruction or to \p Begin
+/// and return the resulting iterator. This function should only be used
+/// MachineBasicBlock::{iterator, const_iterator, instr_iterator,
+/// const_instr_iterator} and the respective reverse iterators.
+template <class IterT>
+inline IterT skipMetaInstructionsBackward(IterT It, IterT Begin) {
+  while (It != Begin && It->isMetaInstruction())
+    It--;
+  return It;
+}
+
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_MACHINEBASICBLOCK_H

llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp

MachineInstr *NewLoad =
           .addMemOperand(
               MBB->getParent()->getMachineMemOperand(LMMO, LMMOffset, Size));
   if (LoadBase.isReg())
     getBaseOperand(NewLoad).setIsKill(false);
   LLVM_DEBUG(NewLoad->dump());
   // If the load and store are consecutive, use the loadInst location to
   // reduce register pressure.
   MachineInstr *StInst = StoreInst;
-  auto PrevInstrIt = skipDebugInstructionsBackward(
+  auto PrevInstrIt = skipMetaInstructionsBackward(
       std::prev(MachineBasicBlock::instr_iterator(StoreInst)),
       MBB->instr_begin());
   if (PrevInstrIt.getNodePtr() == LoadInst)
     StInst = LoadInst;
   MachineInstr *NewStore =
       BuildMI(*MBB, StInst, StInst->getDebugLoc(), TII->get(NStoreOpcode))
           .add(StoreBase)
           .addImm(1)
while (Size > 0) {
     }
   }
   assert(Size == 0 && "Wrong size division");
 }
 
 static void updateKillStatus(MachineInstr *LoadInst, MachineInstr *StoreInst) {
   MachineOperand &LoadBase = getBaseOperand(LoadInst);
   MachineOperand &StoreBase = getBaseOperand(StoreInst);
-  auto StorePrevNonDbgInstr = skipDebugInstructionsBackward(
+  auto StorePrevNonMetaInstr = skipMetaInstructionsBackward(
           std::prev(MachineBasicBlock::instr_iterator(StoreInst)),
           LoadInst->getParent()->instr_begin()).getNodePtr();
   if (LoadBase.isReg()) {
     MachineInstr *LastLoad = LoadInst->getPrevNode();
     // If the original load and store to xmm/ymm were consecutive
     // then the partial copies were also created in
     // a consecutive order to reduce register pressure,
     // and the location of the last load is before the last store.
-    if (StorePrevNonDbgInstr == LoadInst)
+    if (StorePrevNonMetaInstr == LoadInst)
       LastLoad = LoadInst->getPrevNode()->getPrevNode();
     getBaseOperand(LastLoad).setIsKill(LoadBase.isKill());
   }
   if (StoreBase.isReg()) {
     MachineInstr *StInst = StoreInst;
-    if (StorePrevNonDbgInstr == LoadInst)
+    if (StorePrevNonMetaInstr == LoadInst)
       StInst = LoadInst;
     getBaseOperand(StInst->getPrevNode()).setIsKill(StoreBase.isKill());
   }
 }
 
 bool X86AvoidSFBPass::alias(const MachineMemOperand &Op1,
                             const MachineMemOperand &Op2) const {
   if (!Op1.getValue() || !Op2.getValue())

llvm/test/CodeGen/X86/avoid-sfb-ignore-meta.mir

+# RUN: llc %s -run-pass x86-avoid-SFB -mtriple=x86_64-unknown-linux-gnu -o - | FileCheck %s
+#
+# This was generated from:
+#
+# using alpha = float __attribute__((ext_vector_type(4)));
+#
+# void bravo(alpha * __restrict__ p1, alpha * __restrict__ p2) {
+#   char *p3 = (char *)p1;
+#   *p3 = 0;
+#   alpha t = *p1;
+#   *p2 = t;
+# }
+#
+# Using the command line:
+# clang -g -c 1.cpp -O2 -S -emit-llvm -fno-strict-aliasing --target=x86_64-unknown-unknown -o test.ll
+# llc -stop-before=x86-avoid-SFB test.ll -o before.mir
+#
+# The debug instruction between the load and the store has been manually replaced with a CFI meta instruction.
+
+--- |
+  ; ModuleID = 'test.ll'
+  source_filename = "1.cpp"
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-unknown"
+
+  ; Function Attrs: norecurse nounwind uwtable
+  define dso_local void @debug(<4 x float>* noalias nocapture %p1, <4 x float>* noalias nocapture %p2) local_unnamed_addr #0 !dbg !10 {
+  entry:
+    call void @llvm.dbg.value(metadata <4 x float>* %p1, metadata !21, metadata !DIExpression()), !dbg !25
+    call void @llvm.dbg.value(metadata <4 x float>* %p2, metadata !22, metadata !DIExpression()), !dbg !25
+    %0 = bitcast <4 x float>* %p1 to i8*, !dbg !26
+    call void @llvm.dbg.value(metadata i8* %0, metadata !23, metadata !DIExpression()), !dbg !25
+    store i8 0, i8* %0, align 1, !dbg !27
+    %1 = load <4 x float>, <4 x float>* %p1, align 16, !dbg !28
+    call void @llvm.dbg.value(metadata <4 x float> %1, metadata !24, metadata !DIExpression()), !dbg !25
+    store <4 x float> %1, <4 x float>* %p2, align 16, !dbg !29
+    ret void, !dbg !30
+  }
+
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #2
+
+  attributes #0 = { norecurse nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+  attributes #1 = { nounwind readnone speculatable }
+  attributes #2 = { nounwind }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!6, !7, !8}
+  !llvm.ident = !{!9}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 9.0.0 (https://github.com/llvm/llvm-project.git 9afc4764dd24bd2f23c44e51ad33f8e58234a8b6)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+  !1 = !DIFile(filename: "1.cpp", directory: "C:\5CUsers\5Cgbdawsoc\5CDocuments\5Cllvm\5Cbg40969")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !5, size: 64)
+  !5 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+  !6 = !{i32 2, !"Dwarf Version", i32 4}
+  !7 = !{i32 2, !"Debug Info Version", i32 3}
+  !8 = !{i32 1, !"wchar_size", i32 4}
+  !9 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 9afc4764dd24bd2f23c44e51ad33f8e58234a8b6)"}
+  !10 = distinct !DISubprogram(name: "bravo", linkageName: "_Z5bravoPDv4_fS0_", scope: !1, file: !1, line: 4, type: !11, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !20)
+  !11 = !DISubroutineType(types: !12)
+  !12 = !{null, !13, !13}
+  !13 = !DIDerivedType(tag: DW_TAG_restrict_type, baseType: !14)
+  !14 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !15, size: 64)
+  !15 = !DIDerivedType(tag: DW_TAG_typedef, name: "alpha", file: !1, line: 2, baseType: !16)
+  !16 = !DICompositeType(tag: DW_TAG_array_type, baseType: !17, size: 128, flags: DIFlagVector, elements: !18)
+  !17 = !DIBasicType(name: "float", size: 32, encoding: DW_ATE_float)
+  !18 = !{!19}
+  !19 = !DISubrange(count: 4)
+  !20 = !{!21, !22, !23, !24}
+  !21 = !DILocalVariable(name: "p1", arg: 1, scope: !10, file: !1, line: 4, type: !13)
+  !22 = !DILocalVariable(name: "p2", arg: 2, scope: !10, file: !1, line: 4, type: !13)
+  !23 = !DILocalVariable(name: "p3", scope: !10, file: !1, line: 5, type: !4)
+  !24 = !DILocalVariable(name: "t", scope: !10, file: !1, line: 7, type: !15)
+  !25 = !DILocation(line: 0, scope: !10)
+  !26 = !DILocation(line: 5, column: 14, scope: !10)
+  !27 = !DILocation(line: 6, column: 7, scope: !10)
+  !28 = !DILocation(line: 7, column: 13, scope: !10)
+  !29 = !DILocation(line: 8, column: 7, scope: !10)
+  !30 = !DILocation(line: 9, column: 1, scope: !10)
+
+...
+---
+name:            debug
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:
+  - { id: 0, class: gr64, preferred-register: '' }
+  - { id: 1, class: gr64, preferred-register: '' }
+  - { id: 2, class: vr128, preferred-register: '' }
+liveins:
+  - { reg: '$rdi', virtual-reg: '%0' }
+  - { reg: '$rsi', virtual-reg: '%1' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    0
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    liveins: $rdi, $rsi
+
+    DBG_VALUE $rdi, $noreg, !21, !DIExpression(), debug-location !25
+    DBG_VALUE $rsi, $noreg, !22, !DIExpression(), debug-location !25
+    %1:gr64 = COPY $rsi
+    DBG_VALUE %1, $noreg, !22, !DIExpression(), debug-location !25
+    %0:gr64 = COPY $rdi
+    DBG_VALUE %0, $noreg, !21, !DIExpression(), debug-location !25
+    DBG_VALUE %0, $noreg, !23, !DIExpression(), debug-location !25
+    MOV8mi %0, 1, $noreg, 0, $noreg, 0, debug-location !27 :: (store 1 into %ir.0)    
+    %2:vr128 = MOVAPSrm %0, 1, $noreg, 0, $noreg, debug-location !28 :: (load 16 from %ir.p1)
+    CFI_INSTRUCTION offset $r13, -123
+    MOVAPSmr %1, 1, $noreg, 0, $noreg, killed %2, debug-location !29 :: (store 16 into %ir.p2)
+    RET 0, debug-location !30
+
+    ; CHECK-LABEL: name: debug
+    ; CHECK: %1:gr64 = COPY
+    ; CHECK: %0:gr64 = COPY
+    ; CHECK: MOV8mi
+    ; CHECK: %3:gr8 = MOV8rm
+    ; CHECK: MOV8mr
+    ; CHECK: %4:gr64 = MOV64rm
+    ; CHECK: MOV64mr
+    ; CHECK: %5:gr32 = MOV32rm
+    ; CHECK: MOV32mr
+    ; CHECK: %6:gr16 = MOV16rm
+    ; CHECK: MOV16mr
+    ; CHECK: %7:gr8 = MOV8rm
+    ; CHECK: MOV8mr
+    ; CHECK: RET 0
+...