[llvm] r351721 - [AVR] Insert unconditional branch when inserting MBBs between blocks with fallthrough

Dylan McKay via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 20 20:32:02 PST 2019


Author: dylanmckay
Date: Sun Jan 20 20:32:02 2019
New Revision: 351721

URL: http://llvm.org/viewvc/llvm-project?rev=351721&view=rev
Log:
[AVR] Insert unconditional branch when inserting MBBs between blocks with fallthrough

This updates the AVR Select8/Select16 expansion code so that, when
inserting the two basic blocks for true and false conditions, any
existing fallthrough on the previous block is preserved.

Prior to this patch, if the block before the Select pseudo fell through
to the subsequent block, two new basic blocks would be inserted at the
prior fallthrough point, changing the fallthrough destination.

The predecessor or successor lists were not updated, causing the
BranchFolding pass at -O1 and above the rearrange basic blocks, causing
an infinite loop. Not to mention the unconditional fallthrough to the
true block is incorrect in of itself.

This patch modifies the Select8/16 expansion so that, if inserting true
and false basic blocks at a fallthrough point, the implicit branch is
preserved by means of an explicit, unconditional branch to the previous
fallthrough destination.

Thanks to Carl Peto for reporting this bug.

This fixes avr-rust bug https://github.com/avr-rust/rust/issues/123.

Added:
    llvm/trunk/test/CodeGen/AVR/avr-rust-issue-123.ll
Modified:
    llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp

Modified: llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp?rev=351721&r1=351720&r2=351721&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp Sun Jan 20 20:32:02 2019
@@ -1634,6 +1634,15 @@ AVRTargetLowering::EmitInstrWithCustomIn
 
   MachineFunction *MF = MBB->getParent();
   const BasicBlock *LLVM_BB = MBB->getBasicBlock();
+  MachineBasicBlock *FallThrough = MBB->getFallThrough();
+
+  // If the current basic block falls through to another basic block,
+  // we must insert an unconditional branch to the fallthrough destination
+  // if we are to insert basic blocks at the prior fallthrough point.
+  if (FallThrough != nullptr) {
+    BuildMI(MBB, dl, TII.get(AVR::RJMPk)).addMBB(FallThrough);
+  }
+
   MachineBasicBlock *trueMBB = MF->CreateMachineBasicBlock(LLVM_BB);
   MachineBasicBlock *falseMBB = MF->CreateMachineBasicBlock(LLVM_BB);
 

Added: llvm/trunk/test/CodeGen/AVR/avr-rust-issue-123.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/avr-rust-issue-123.ll?rev=351721&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/avr-rust-issue-123.ll (added)
+++ llvm/trunk/test/CodeGen/AVR/avr-rust-issue-123.ll Sun Jan 20 20:32:02 2019
@@ -0,0 +1,74 @@
+; RUN: llc -O1 < %s -march=avr | FileCheck %s
+
+; This test ensures that the Select8/Select16 expansion
+; pass inserts an unconditional branch to the previous adjacent
+; basic block when inserting new basic blocks when the
+; prior block has a fallthrough.
+;
+; Before this bug was fixed, Select8/Select16 expansion
+; would leave a dangling fallthrough to an undefined block.
+;
+; The BranchFolding pass would later rearrange the basic
+; blocks based on predecessor/successor list assumptions
+; which were made incorrect due to the invalid Select
+; expansion.
+
+; More information in
+; https://github.com/avr-rust/rust/issues/123.
+
+%UInt8 = type <{ i8 }>
+%UInt32 = type <{ i32 }>
+%Sb = type <{ i1 }>
+
+ at delayFactor = hidden global %UInt8 zeroinitializer, align 1
+ at delay = hidden global %UInt32 zeroinitializer, align 4
+ at flag = hidden global %Sb zeroinitializer, align 1
+
+declare void @eeprom_write(i16, i8)
+
+; CHECK-LABEL: update_register
+define hidden void @update_register(i8 %arg, i8 %arg1) {
+entry:
+  ; CHECK: push [[PRELUDER:r[0-9]+]]
+  ; CHECK: cpi  r24, 7
+  switch i8 %arg, label %bb7 [
+    i8 6, label %bb
+    i8 7, label %bb6
+  ]
+
+; CHECK-NOT: ret
+bb:                                               ; preds = %entry
+  %tmp = icmp ugt i8 %arg1, 90
+  %tmp2 = icmp ult i8 %arg1, 5
+  %. = select i1 %tmp2, i8 5, i8 %arg1
+  %tmp3 = select i1 %tmp, i8 90, i8 %.
+  ; CHECK: sts delayFactor, r{{[0-9]+}}
+  store i8 %tmp3, i8* getelementptr inbounds (%UInt8, %UInt8* @delayFactor, i64 0, i32 0), align 1
+  %tmp4 = zext i8 %tmp3 to i32
+  %tmp5 = mul nuw nsw i32 %tmp4, 100
+  ; CHECK:      sts  delay+3, r{{[0-9]+}}
+  ; CHECK-NEXT: sts  delay+2, r{{[0-9]+}}
+  ; CHECK-NEXT: sts  delay+1, r{{[0-9]+}}
+  ; CHECK-NEXT: sts  delay, r{{[0-9]+}}
+  store i32 %tmp5, i32* getelementptr inbounds (%UInt32, %UInt32* @delay, i64 0, i32 0), align 4
+  tail call void @eeprom_write(i16 34, i8 %tmp3)
+  br label %bb7
+
+bb6:                                              ; preds = %entry
+  %not. = icmp ne i8 %arg1, 0
+  %.2 = zext i1 %not. to i8
+  store i1 %not., i1* getelementptr inbounds (%Sb, %Sb* @flag, i64 0, i32 0), align 1
+
+  ; CHECK: call eeprom_write
+  tail call void @eeprom_write(i16 35, i8 %.2)
+  br label %bb7
+
+  ; CHECK: LBB0_{{[0-9]+}}
+  ; CHECK: pop [[PRELUDER]]
+  ; CHECK-NEXT: ret
+bb7:                                              ; preds = %bb6, %bb, %entry
+  ret void
+}
+; CHECK-NOT: LBB0_{{[0-9]+}}:
+; CHECK-LABEL: .Lfunc_end0
+; CHECK: .size  update_register, .Lfunc_end0-update_register




More information about the llvm-commits mailing list