[llvm] 1a7398e - [BranchAlign] Add master --x86-branches-within-32B-boundaries flag

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2020-01-14T18:17:53-08:00
New Revision: 1a7398eca2040d232149f18a75b5d78a6521941c

URL: https://github.com/llvm/llvm-project/commit/1a7398eca2040d232149f18a75b5d78a6521941c
DIFF: https://github.com/llvm/llvm-project/commit/1a7398eca2040d232149f18a75b5d78a6521941c.diff

LOG: [BranchAlign] Add master --x86-branches-within-32B-boundaries flag

This flag was originally part of D70157, but was removed as we carved away pieces of the review. Since we have the nop support checked in, and it appears mature(*), I think it's time to add the master flag. For now, it will default to nop padding, but once the prefix padding support lands, we'll update the defaults.

(*) I can now confirm that downstream testing of the changes which have landed to date - nop padding and compiler support for suppressions - is passing all of the functional testing we've thrown at it. There might still be something lurking, but we've gotten enough coverage to be confident of the basic approach.

Note that the new flag can be used either when assembling an .s file, or when using the integrated assembler directly from the compiler. The later will use all of the suppression mechanism and should always generate correct code. We don't yet have assembly syntax for the suppressions, so passing this directly to the assembler w/a raw .s file may result in broken code. Use at your own risk.

Also note that this isn't the wiring for the clang option. I think the most recent review for that is D72227, but I've lost track, so that might be off.

Differential Revision: https://reviews.llvm.org/D72738

Added: 
    

Modified: 
    llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
    llvm/test/MC/X86/align-branch-64-1a.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index da280547c4b3..dffda5217675 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -94,6 +94,14 @@ cl::opt<X86AlignBranchKind, true, cl::parser<std::string>> X86AlignBranch(
                    "indirect indicates indirect jumps."),
     cl::location(X86AlignBranchKindLoc));
 
+cl::opt<bool> X86AlignBranchWithin32BBoundaries(
+    "x86-branches-within-32B-boundaries", cl::init(false),
+    cl::desc(
+        "Align selected instructions to mitigate negative performance impact "
+        "of Intel's micro code update for errata skx102.  May break "
+        "assumptions about labels corresponding to particular instructions, "
+        "and should be used with caution."));
+
 class X86ELFObjectWriter : public MCELFObjectTargetWriter {
 public:
   X86ELFObjectWriter(bool is64Bit, uint8_t OSABI, uint16_t EMachine,
@@ -119,8 +127,21 @@ class X86AsmBackend : public MCAsmBackend {
   X86AsmBackend(const Target &T, const MCSubtargetInfo &STI)
       : MCAsmBackend(support::little), STI(STI),
         MCII(T.createMCInstrInfo()) {
-    AlignBoundary = assumeAligned(X86AlignBranchBoundary);
-    AlignBranchType = X86AlignBranchKindLoc;
+    if (X86AlignBranchWithin32BBoundaries) {
+      // At the moment, this defaults to aligning fused branches, unconditional
+      // jumps, and (unfused) conditional jumps with nops.  Both the
+      // instructions aligned and the alignment method (nop vs prefix) may
+      // change in the future.
+      AlignBoundary = assumeAligned(32);;
+      AlignBranchType.addKind(X86::AlignBranchFused);
+      AlignBranchType.addKind(X86::AlignBranchJcc);
+      AlignBranchType.addKind(X86::AlignBranchJmp);
+    }
+    // Allow overriding defaults set by master flag
+    if (X86AlignBranchBoundary.getNumOccurrences())
+      AlignBoundary = assumeAligned(X86AlignBranchBoundary);
+    if (X86AlignBranch.getNumOccurrences())
+      AlignBranchType = X86AlignBranchKindLoc;
   }
 
   bool allowAutoPadding() const override;

diff  --git a/llvm/test/MC/X86/align-branch-64-1a.s b/llvm/test/MC/X86/align-branch-64-1a.s
index 75381568d227..49f1406e0947 100644
--- a/llvm/test/MC/X86/align-branch-64-1a.s
+++ b/llvm/test/MC/X86/align-branch-64-1a.s
@@ -1,11 +1,14 @@
 # Check only fused conditional jumps, conditional jumps and unconditional jumps are aligned with option --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp
-# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp %p/Inputs/align-branch-64-1.s | llvm-objdump -d  - > %t1
-# RUN: FileCheck --input-file=%t1 %s
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp %p/Inputs/align-branch-64-1.s -o %t1 && llvm-objdump -d %t1 | FileCheck %s
+
+# Check that -x86-branches-within-32B-boundaries matches the above command line
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-branches-within-32B-boundaries %p/Inputs/align-branch-64-1.s -o %t2
+# RUN: cmp %t1 %t2
 
 # Check no branches is aligned with option --x86-align-branch-boundary=0
-# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=0 --x86-align-branch=fused+jcc+jmp %p/Inputs/align-branch-64-1.s | llvm-objdump -d  - > %t2
-# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %p/Inputs/align-branch-64-1.s | llvm-objdump -d  - > %t3
-# RUN: cmp %t2 %t3
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=0 --x86-align-branch=fused+jcc+jmp %p/Inputs/align-branch-64-1.s | llvm-objdump -d  - > %t3
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %p/Inputs/align-branch-64-1.s | llvm-objdump -d  - > %t4
+# RUN: cmp %t3 %t4
 
 # CHECK: 0000000000000000 foo:
 # CHECK-COUNT-3:      : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1