From 2632c677f85cba1ac2aef5d68aaf8af0f5b3c944 Mon Sep 17 00:00:00 2001
From: Paul Walker <paul.walker@arm.com>
Date: Fri, 16 Aug 2019 17:29:53 +0000
Subject: [PATCH] [AArch64InstrInfo] Stop getInstSizeInBytes returning non-zero
 for meta instructions.

Recommit with fixes for mac builders.

Summary:
AArch64InstrInfo::getInstSizeInBytes is incorrectly treating meta
instructions (e.g. CFI_INSTRUCTION) as normal instructions and
giving them a size of 4.

This results in branch relaxation calculating block sizes wrong.
Branch relaxation also considers alignment and thus a single
mistake can result in later blocks being incorrectly sized even
when they themselves do not contain meta instructions.

The net result is we might not relax a branch whose destination is
not within range.

Reviewers: nickdesaulniers, peter.smith

Reviewed By: peter.smith

Subscribers: javed.absar, kristof.beyls, llvm-commits

Tags: #llvm

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

> llvm-svn: 369111

llvm-svn: 369133
---
 llvm/lib/Target/AArch64/AArch64InstrInfo.cpp       | 10 ++-
 .../CodeGen/AArch64/branch-relax-block-size.mir    | 80 ++++++++++++++++++++++
 2 files changed, 84 insertions(+), 6 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/branch-relax-block-size.mir

diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index ce5a008..1e089ff 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -83,6 +83,10 @@ unsigned AArch64InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
       return getInlineAsmLength(MI.getOperand(0).getSymbolName(), *MAI);
   }
 
+  // Meta-instructions emit no code.
+  if (MI.isMetaInstruction())
+    return 0;
+
   // FIXME: We currently only handle pseudoinstructions that don't get expanded
   //        before the assembly printer.
   unsigned NumBytes = 0;
@@ -92,12 +96,6 @@ unsigned AArch64InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
     // Anything not explicitly designated otherwise is a normal 4-byte insn.
     NumBytes = 4;
     break;
-  case TargetOpcode::DBG_VALUE:
-  case TargetOpcode::EH_LABEL:
-  case TargetOpcode::IMPLICIT_DEF:
-  case TargetOpcode::KILL:
-    NumBytes = 0;
-    break;
   case TargetOpcode::STACKMAP:
     // The upper bound for a stackmap intrinsic is the full length of its shadow
     NumBytes = StackMapOpers(&MI).getNumPatchBytes();
diff --git a/llvm/test/CodeGen/AArch64/branch-relax-block-size.mir b/llvm/test/CodeGen/AArch64/branch-relax-block-size.mir
new file mode 100644
index 0000000..2670580
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/branch-relax-block-size.mir
@@ -0,0 +1,80 @@
+# REQUIRES: asserts
+# RUN: llc -mtriple=aarch64--linux-gnu -run-pass=branch-relaxation -debug-only=branch-relaxation %s -o /dev/null 2>&1 | FileCheck %s
+
+# Ensure meta instructions (e.g. CFI_INSTRUCTION) don't contribute to the code
+# size of a basic block.
+
+# CHECK:  Basic blocks before relaxation
+# CHECK-NEXT: bb.0 offset=00000000 size=0x18
+# CHECK-NEXT: bb.1 offset=00000018 size=0x4
+# CHECK-NEXT: bb.2 offset=0000001c size=0xc
+
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--linux-gnu"
+
+  define i32 @test(i32* %a) #0 {
+  entry:
+    %call = tail call i32 @validate(i32* %a)
+    %tobool = icmp eq i32 %call, 0
+    br i1 %tobool, label %return, label %if.then
+
+  if.then:                                          ; preds = %entry
+    %0 = load i32, i32* %a, align 4
+    br label %return
+
+  return:                                           ; preds = %entry, %if.then
+    %retval.0 = phi i32 [ %0, %if.then ], [ 0, %entry ]
+    ret i32 %retval.0
+  }
+
+  declare i32 @validate(i32*)
+
+  attributes #0 = { nounwind uwtable "disable-tail-calls"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" }
+
+...
+---
+name:            test
+alignment:       2
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0' }
+frameInfo:
+  stackSize:       32
+  maxAlignment:    16
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+stack:
+  - { id: 0, type: spill-slot, offset: -8, size: 8, alignment: 8, callee-saved-register: '$lr' }
+  - { id: 1, type: spill-slot, offset: -16, size: 8, alignment: 8, callee-saved-register: '$fp' }
+  - { id: 2, type: spill-slot, offset: -32, size: 8, alignment: 16, callee-saved-register: '$x19' }
+body:             |
+  bb.0.entry:
+    successors: %bb.2(0x30000000), %bb.1(0x50000000)
+    liveins: $x0, $x19, $lr
+
+    early-clobber $sp = frame-setup STRXpre killed $x19, $sp, -32 :: (store 8 into %stack.2)
+    frame-setup STPXi $fp, killed $lr, $sp, 2 :: (store 8 into %stack.1), (store 8 into %stack.0)
+    $fp = frame-setup ADDXri $sp, 16, 0
+    frame-setup CFI_INSTRUCTION def_cfa $w29, 16
+    frame-setup CFI_INSTRUCTION offset $w30, -8
+    frame-setup CFI_INSTRUCTION offset $w29, -16
+    frame-setup CFI_INSTRUCTION offset $w19, -32
+    $x19 = ORRXrs $xzr, $x0, 0
+    BL @validate, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit-def $sp, implicit-def $w0
+    CBZW renamable $w0, %bb.2
+
+  bb.1.if.then:
+    liveins: $x19
+
+    renamable $w0 = LDRWui killed renamable $x19, 0 :: (load 4 from %ir.a)
+
+  bb.2.return:
+    liveins: $w0
+
+    $fp, $lr = frame-destroy LDPXi $sp, 2 :: (load 8 from %stack.1), (load 8 from %stack.0)
+    early-clobber $sp, $x19 = frame-destroy LDRXpost $sp, 32 :: (load 8 from %stack.2)
+    RET undef $lr, implicit killed $w0
+
+...
-- 
2.7.4