Fix for bug 34002

[maya.madhavan]

Summary

When a label is encountered while an open 'it' block is under construction (under construction because more conditional instructions can be added to it), the gathered block should be emitted before the label is emitted.
Due to the location from where the onLabelParsed function was being called, the label is emitted first and the 'it' block flushed after, resulting in incorrect code.

[maya.madhavan]

Verification

1. Rebuilt with changes.
2. Output of test noted in bug:

.text
.p2align        1
.code   16
.globl  f1
.globl  f2

f1:

.thumb_func
cmp     r0, #10



it      pl
movpl   r0, #0

f2:

.thumb_func
movs    r1, #0

.Ltmp:

cmp     r0, #0
ittt    pl
addpl   r1, r1, r0
subpl   r0, r0, #1
bpl     .Ltmp
mov     r0, r1
bx      lr

The label 'f2' is in the expected location now.

3. make check (in progress)

[olista01]

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

9451–9454

Can this call be removed now? doBeforeLabelEmit and onLabelParsed are only ever called in pairs before and after emitting the label, so there should never be anything to flush here, and as you've pointed out doing it here is incorrect.

test/MC/ARM/implicit-it-generation.s

70

I think this test already exposes the bug, we just need to add CHECK lines to make sure the labels are emitted in the correct place. That would be clearer than adding a second test to check the label locations.

[maya.madhavan]

Updated test case and call per Oliver Stannard's suggestion.

[maya.madhavan]

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

9451–9454

Fixed, thanks!

test/MC/ARM/implicit-it-generation.s

70

Updated - thanks!

[olista01]

LGTM, thanks!

include/llvm/MC/MCParser/MCTargetAsmParser.h

public:
   virtual bool starIsStartOfStatement() { return false; };
 
   virtual const MCExpr *applyModifierToExpr(const MCExpr *E,
                                             MCSymbolRefExpr::VariantKind,
                                             MCContext &Ctx) {
     return nullptr;
   }
 
+  // For actions that have to be performed before a label is emitted
+  virtual void doBeforeLabelEmit(MCSymbol *Symbol) {}
+  
   virtual void onLabelParsed(MCSymbol *Symbol) {}
 
   /// Ensure that all previously parsed instructions have been emitted to the
   /// output streamer, if the target does not emit them immediately.
   virtual void flushPendingInstructions(MCStreamer &Out) {}
 
   virtual const MCExpr *createTargetUnaryExpr(const MCExpr *E,
                                               AsmToken::TokenKind OperatorToken,
                                               MCContext &Ctx) {
     return nullptr;
   }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCPARSER_MCTARGETASMPARSER_H

lib/MC/MCParser/AsmParser.cpp

case AsmToken::Colon: {
     }
 
     // Consume any end of statement token, if present, to avoid spurious
     // AddBlankLine calls().
     if (getTok().is(AsmToken::EndOfStatement)) {
       Lex();
     }
 
+    getTargetParser().doBeforeLabelEmit(Sym);
+
     // Emit the label.
     if (!getTargetParser().isParsingInlineAsm())
       Out.EmitLabel(Sym, IDLoc);
 
     // If we are generating dwarf for assembly source files then gather the
     // info to make a dwarf label entry for this label if needed.
     if (enabledGenDwarfForAssembly())
       MCGenDwarfLabelEntry::Make(Sym, &getStreamer(), getSourceManager(),

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

#include "ARMGenAsmMatcher.inc"
   const char *getCustomOperandDiag(ARMMatchResultTy MatchError);
 
   void FilterNearMisses(SmallVectorImpl<NearMissInfo> &NearMissesIn,
                         SmallVectorImpl<NearMissMessage> &NearMissesOut,
                         SMLoc IDLoc, OperandVector &Operands);
   void ReportNearMisses(SmallVectorImpl<NearMissInfo> &NearMisses, SMLoc IDLoc,
                         OperandVector &Operands);
 
+  void doBeforeLabelEmit(MCSymbol *Symbol) override;
+
   void onLabelParsed(MCSymbol *Symbol) override;
 };
 
 /// ARMOperand - Instances of this class represent a parsed ARM machine
 /// operand.
 class ARMOperand : public MCParsedAsmOperand {
   enum KindTy {
     k_CondCode,
if (parseToken(AsmToken::EndOfStatement, "unexpected token in directive") ||
     return true;
 
   if (isThumb())
     SwitchMode();
   getParser().getStreamer().EmitAssemblerFlag(MCAF_Code32);
   return false;
 }
 
-void ARMAsmParser::onLabelParsed(MCSymbol *Symbol) {
+void ARMAsmParser::doBeforeLabelEmit(MCSymbol *Symbol) {
   // We need to flush the current implicit IT block on a label, because it is
   // not legal to branch into an IT block.
   flushPendingInstructions(getStreamer());
+}
+
+void ARMAsmParser::onLabelParsed(MCSymbol *Symbol) {
   if (NextSymbolIsThumb) {
     getParser().getStreamer().EmitThumbFunc(Symbol);
     NextSymbolIsThumb = false;
   }
 }
 
 /// parseDirectiveThumbFunc
 ///  ::= .thumbfunc symbol_name

test/MC/ARM/implicit-it-generation.s

 @ CHECK: addeq
 @ CHECK: addeq
 @ CHECK: addeq
 @ CHECK: addeq
 @ CHECK: itt eq
 @ CHECK: addeq
 @ CHECK: addeq
 
 @ Flush on a label
   .section test6
 @ CHECK-LABEL: test6
   addeq r0, #1
 label:
   addeq r0, #1
-5:
+five:
   addeq r0, #1
 @ CHECK: it eq
 @ CHECK: addeq
+@ CHECK: label
 @ CHECK: it eq
 @ CHECK: addeq
+@ CHECK: five
 @ CHECK: it eq
 @ CHECK: addeq
 
 @ Flush on a section-change directive
   .section test7a
 @ CHECK-LABEL: test7a
   addeq r0, #1
   .section test7b