Skip to content

Commit 6c0665e

Browse files
committedApr 30, 2018
[MC] Change AsmParser to leverage Assembler during evaluation
Teach AsmParser to check with Assembler for when evaluating constant expressions. This improves the handing of preprocessor expressions that must be resolved at parse time. This idiom can be found as assembling-time assertion checks in source-level assemblers. Note that this relies on the MCStreamer to keep sufficient tabs on Section / Fragment information which the MCAsmStreamer does not. As a result the textual output may fail where the equivalent object generation would pass. This can most easily be resolved by folding the MCAsmStreamer and MCObjectStreamer together which is planned for in a separate patch. Currently, this feature is only enabled for assembly input, keeping IR compilation consistent between assembly and object generation. Reviewers: echristo, rnk, probinson, espindola, peter.smith Reviewed By: peter.smith Subscribers: eraman, peter.smith, arichardson, jyknight, hiraditya, llvm-commits Differential Revision: https://reviews.llvm.org/D45164 llvm-svn: 331218
1 parent 8fe04ad commit 6c0665e

17 files changed

+153
-23
lines changed
 

‎clang/test/CodeGen/asm-parser-info.S

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// REQUIRES: x86-registered-target
2+
// RUN: %clang --target=x86_64-unknown-linux-gnu -c %s -o /dev/null
3+
4+
// Check that cc1as can use assembler info in object generation.
5+
.data
6+
7+
foo:
8+
.if . - foo == 0
9+
.byte 0xaa
10+
.else
11+
.byte 0x00
12+
.endif

‎clang/tools/driver/cc1as_main.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,9 @@ static bool ExecuteAssembler(AssemblerInvocation &Opts,
435435
Str.get()->InitSections(Opts.NoExecStack);
436436
}
437437

438+
// Assembly to object compilation should leverage assembly info.
439+
Str->setUseAssemblerInfoForParsing(true);
440+
438441
bool Failed = false;
439442

440443
std::unique_ptr<MCAsmParser> Parser(

‎llvm/include/llvm/MC/MCExpr.h

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ class MCExpr {
9696
const SectionAddrMap &Addrs) const;
9797
bool evaluateAsAbsolute(int64_t &Res) const;
9898
bool evaluateAsAbsolute(int64_t &Res, const MCAssembler &Asm) const;
99+
bool evaluateAsAbsolute(int64_t &Res, const MCAssembler *Asm) const;
99100
bool evaluateAsAbsolute(int64_t &Res, const MCAsmLayout &Layout) const;
100101

101102
bool evaluateKnownAbsolute(int64_t &Res, const MCAsmLayout &Layout) const;

‎llvm/include/llvm/MC/MCObjectStreamer.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class MCObjectStreamer : public MCStreamer {
8989
void visitUsedSymbol(const MCSymbol &Sym) override;
9090

9191
MCAssembler &getAssembler() { return *Assembler; }
92-
92+
MCAssembler *getAssemblerPtr() override;
9393
/// \name MCStreamer Interface
9494
/// @{
9595

‎llvm/include/llvm/MC/MCStreamer.h

+7
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ class MCStreamer {
211211
/// requires.
212212
unsigned NextWinCFIID = 0;
213213

214+
bool UseAssemblerInfoForParsing;
215+
214216
protected:
215217
MCStreamer(MCContext &Ctx);
216218

@@ -247,6 +249,11 @@ class MCStreamer {
247249

248250
MCContext &getContext() const { return Context; }
249251

252+
virtual MCAssembler *getAssemblerPtr() { return nullptr; }
253+
254+
void setUseAssemblerInfoForParsing(bool v) { UseAssemblerInfoForParsing = v; }
255+
bool getUseAssemblerInfoForParsing() { return UseAssemblerInfoForParsing; }
256+
250257
MCTargetStreamer *getTargetStreamer() {
251258
return TargetStreamer.get();
252259
}

‎llvm/include/llvm/MC/MCSymbol.h

+2
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ class MCSymbol {
316316
Index = Value;
317317
}
318318

319+
bool isUnset() const { return SymbolContents == SymContentsUnset; }
320+
319321
uint64_t getOffset() const {
320322
assert((SymbolContents == SymContentsUnset ||
321323
SymbolContents == SymContentsOffset) &&

‎llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
132132
std::unique_ptr<MCAsmParser> Parser(
133133
createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
134134

135+
// Do not use assembler-level information for parsing inline assembly.
136+
OutStreamer->setUseAssemblerInfoForParsing(false);
137+
135138
// We create a new MCInstrInfo here since we might be at the module level
136139
// and not have a MachineFunction to initialize the TargetInstrInfo from and
137140
// we only need MCInstrInfo for asm parsing. We create one unconditionally

‎llvm/lib/MC/MCAsmStreamer.cpp

+7-4
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ class MCAsmStreamer final : public MCStreamer {
7878
InstPrinter->setCommentStream(CommentStream);
7979
}
8080

81+
MCAssembler &getAssembler() { return *Assembler; }
82+
MCAssembler *getAssemblerPtr() override { return nullptr; }
83+
8184
inline void EmitEOL() {
8285
// Dump Explicit Comments here.
8386
emitExplicitComments();
@@ -1656,10 +1659,10 @@ void MCAsmStreamer::AddEncodingComment(const MCInst &Inst,
16561659
raw_svector_ostream VecOS(Code);
16571660

16581661
// If we have no code emitter, don't emit code.
1659-
if (!Assembler->getEmitterPtr())
1662+
if (!getAssembler().getEmitterPtr())
16601663
return;
16611664

1662-
Assembler->getEmitter().encodeInstruction(Inst, VecOS, Fixups, STI);
1665+
getAssembler().getEmitter().encodeInstruction(Inst, VecOS, Fixups, STI);
16631666

16641667
// If we are showing fixups, create symbolic markers in the encoded
16651668
// representation. We do this by making a per-bit map to the fixup item index,
@@ -1672,7 +1675,7 @@ void MCAsmStreamer::AddEncodingComment(const MCInst &Inst,
16721675
for (unsigned i = 0, e = Fixups.size(); i != e; ++i) {
16731676
MCFixup &F = Fixups[i];
16741677
const MCFixupKindInfo &Info =
1675-
Assembler->getBackend().getFixupKindInfo(F.getKind());
1678+
getAssembler().getBackend().getFixupKindInfo(F.getKind());
16761679
for (unsigned j = 0; j != Info.TargetSize; ++j) {
16771680
unsigned Index = F.getOffset() * 8 + Info.TargetOffset + j;
16781681
assert(Index < Code.size() * 8 && "Invalid offset in fixup!");
@@ -1737,7 +1740,7 @@ void MCAsmStreamer::AddEncodingComment(const MCInst &Inst,
17371740
for (unsigned i = 0, e = Fixups.size(); i != e; ++i) {
17381741
MCFixup &F = Fixups[i];
17391742
const MCFixupKindInfo &Info =
1740-
Assembler->getBackend().getFixupKindInfo(F.getKind());
1743+
getAssembler().getBackend().getFixupKindInfo(F.getKind());
17411744
OS << " fixup " << char('A' + i) << " - " << "offset: " << F.getOffset()
17421745
<< ", value: " << *F.getValue() << ", kind: " << Info.Name << "\n";
17431746
}

‎llvm/lib/MC/MCExpr.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,10 @@ bool MCExpr::evaluateAsAbsolute(int64_t &Res, const MCAssembler &Asm) const {
441441
return evaluateAsAbsolute(Res, &Asm, nullptr, nullptr);
442442
}
443443

444+
bool MCExpr::evaluateAsAbsolute(int64_t &Res, const MCAssembler *Asm) const {
445+
return evaluateAsAbsolute(Res, Asm, nullptr, nullptr);
446+
}
447+
444448
bool MCExpr::evaluateKnownAbsolute(int64_t &Res,
445449
const MCAsmLayout &Layout) const {
446450
return evaluateAsAbsolute(Res, &Layout.getAssembler(), &Layout, nullptr,
@@ -494,7 +498,7 @@ static void AttemptToFoldSymbolOffsetDifference(
494498
return;
495499

496500
if (SA.getFragment() == SB.getFragment() && !SA.isVariable() &&
497-
!SB.isVariable()) {
501+
!SA.isUnset() && !SB.isVariable() && !SB.isUnset()) {
498502
Addend += (SA.getOffset() - SB.getOffset());
499503

500504
// Pointers to Thumb symbols need to have their low-bit set to allow

‎llvm/lib/MC/MCObjectStreamer.cpp

+16-7
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ MCObjectStreamer::MCObjectStreamer(MCContext &Context,
3535

3636
MCObjectStreamer::~MCObjectStreamer() {}
3737

38+
// AssemblerPtr is used for evaluation of expressions and causes
39+
// difference between asm and object outputs. Return nullptr to in
40+
// inline asm mode to limit divergence to assembly inputs.
41+
MCAssembler *MCObjectStreamer::getAssemblerPtr() {
42+
if (getUseAssemblerInfoForParsing())
43+
return Assembler.get();
44+
return nullptr;
45+
}
46+
3847
void MCObjectStreamer::flushPendingLabels(MCFragment *F, uint64_t FOffset) {
3948
if (PendingLabels.empty())
4049
return;
@@ -155,7 +164,7 @@ void MCObjectStreamer::EmitValueImpl(const MCExpr *Value, unsigned Size,
155164

156165
// Avoid fixups when possible.
157166
int64_t AbsValue;
158-
if (Value->evaluateAsAbsolute(AbsValue, getAssembler())) {
167+
if (Value->evaluateAsAbsolute(AbsValue, getAssemblerPtr())) {
159168
if (!isUIntN(8 * Size, AbsValue) && !isIntN(8 * Size, AbsValue)) {
160169
getContext().reportError(
161170
Loc, "value evaluated as " + Twine(AbsValue) + " is out of range.");
@@ -217,7 +226,7 @@ void MCObjectStreamer::EmitLabel(MCSymbol *Symbol, SMLoc Loc, MCFragment *F) {
217226

218227
void MCObjectStreamer::EmitULEB128Value(const MCExpr *Value) {
219228
int64_t IntValue;
220-
if (Value->evaluateAsAbsolute(IntValue, getAssembler())) {
229+
if (Value->evaluateAsAbsolute(IntValue, getAssemblerPtr())) {
221230
EmitULEB128IntValue(IntValue);
222231
return;
223232
}
@@ -226,7 +235,7 @@ void MCObjectStreamer::EmitULEB128Value(const MCExpr *Value) {
226235

227236
void MCObjectStreamer::EmitSLEB128Value(const MCExpr *Value) {
228237
int64_t IntValue;
229-
if (Value->evaluateAsAbsolute(IntValue, getAssembler())) {
238+
if (Value->evaluateAsAbsolute(IntValue, getAssemblerPtr())) {
230239
EmitSLEB128IntValue(IntValue);
231240
return;
232241
}
@@ -254,7 +263,7 @@ bool MCObjectStreamer::changeSectionImpl(MCSection *Section,
254263

255264
int64_t IntSubsection = 0;
256265
if (Subsection &&
257-
!Subsection->evaluateAsAbsolute(IntSubsection, getAssembler()))
266+
!Subsection->evaluateAsAbsolute(IntSubsection, getAssemblerPtr()))
258267
report_fatal_error("Cannot evaluate subsection number");
259268
if (IntSubsection < 0 || IntSubsection > 8192)
260269
report_fatal_error("Subsection number out of range");
@@ -400,7 +409,7 @@ void MCObjectStreamer::EmitDwarfAdvanceLineAddr(int64_t LineDelta,
400409
}
401410
const MCExpr *AddrDelta = buildSymbolDiff(*this, Label, LastLabel);
402411
int64_t Res;
403-
if (AddrDelta->evaluateAsAbsolute(Res, getAssembler())) {
412+
if (AddrDelta->evaluateAsAbsolute(Res, getAssemblerPtr())) {
404413
MCDwarfLineAddr::Emit(this, Assembler->getDWARFLinetableParams(), LineDelta,
405414
Res);
406415
return;
@@ -412,7 +421,7 @@ void MCObjectStreamer::EmitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
412421
const MCSymbol *Label) {
413422
const MCExpr *AddrDelta = buildSymbolDiff(*this, Label, LastLabel);
414423
int64_t Res;
415-
if (AddrDelta->evaluateAsAbsolute(Res, getAssembler())) {
424+
if (AddrDelta->evaluateAsAbsolute(Res, getAssemblerPtr())) {
416425
MCDwarfFrameEmitter::EmitAdvanceLoc(*this, Res);
417426
return;
418427
}
@@ -608,7 +617,7 @@ void MCObjectStreamer::emitFill(const MCExpr &NumBytes, uint64_t FillValue,
608617
void MCObjectStreamer::emitFill(const MCExpr &NumValues, int64_t Size,
609618
int64_t Expr, SMLoc Loc) {
610619
int64_t IntNumValues;
611-
if (!NumValues.evaluateAsAbsolute(IntNumValues, getAssembler())) {
620+
if (!NumValues.evaluateAsAbsolute(IntNumValues, getAssemblerPtr())) {
612621
getContext().reportError(Loc, "expected absolute expression");
613622
return;
614623
}

‎llvm/lib/MC/MCParser/AsmParser.cpp

+10-7
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,7 @@ bool AsmParser::processIncbinFile(const std::string &Filename, int64_t Skip,
775775
Bytes = Bytes.drop_front(Skip);
776776
if (Count) {
777777
int64_t Res;
778-
if (!Count->evaluateAsAbsolute(Res))
778+
if (!Count->evaluateAsAbsolute(Res, getStreamer().getAssemblerPtr()))
779779
return Error(Loc, "expected absolute expression");
780780
if (Res < 0)
781781
return Warning(Loc, "negative count has no effect");
@@ -1378,7 +1378,8 @@ bool AsmParser::parseExpression(const MCExpr *&Res, SMLoc &EndLoc) {
13781378
Lex();
13791379
}
13801380

1381-
// Try to constant fold it up front, if possible.
1381+
// Try to constant fold it up front, if possible. Do not exploit
1382+
// assembler here.
13821383
int64_t Value;
13831384
if (Res->evaluateAsAbsolute(Value))
13841385
Res = MCConstantExpr::create(Value, getContext());
@@ -1419,7 +1420,7 @@ bool AsmParser::parseAbsoluteExpression(int64_t &Res) {
14191420
if (parseExpression(Expr))
14201421
return true;
14211422

1422-
if (!Expr->evaluateAsAbsolute(Res))
1423+
if (!Expr->evaluateAsAbsolute(Res, getStreamer().getAssemblerPtr()))
14231424
return Error(StartLoc, "expected absolute expression");
14241425

14251426
return false;
@@ -2616,7 +2617,8 @@ bool AsmParser::parseMacroArguments(const MCAsmMacro *M,
26162617
Lex();
26172618
if (parseExpression(AbsoluteExp, EndLoc))
26182619
return false;
2619-
if (!AbsoluteExp->evaluateAsAbsolute(Value))
2620+
if (!AbsoluteExp->evaluateAsAbsolute(Value,
2621+
getStreamer().getAssemblerPtr()))
26202622
return Error(StrLoc, "expected absolute expression");
26212623
const char *StrChar = StrLoc.getPointer();
26222624
const char *EndChar = EndLoc.getPointer();
@@ -2916,8 +2918,9 @@ bool AsmParser::parseDirectiveReloc(SMLoc DirectiveLoc) {
29162918
if (parseExpression(Offset))
29172919
return true;
29182920

2919-
if (check(!Offset->evaluateAsAbsolute(OffsetValue), OffsetLoc,
2920-
"expression is not a constant value") ||
2921+
if (check(!Offset->evaluateAsAbsolute(OffsetValue,
2922+
getStreamer().getAssemblerPtr()),
2923+
OffsetLoc, "expression is not a constant value") ||
29212924
check(OffsetValue < 0, OffsetLoc, "expression is negative") ||
29222925
parseToken(AsmToken::Comma, "expected comma") ||
29232926
check(getTok().isNot(AsmToken::Identifier), "expected relocation name"))
@@ -5344,7 +5347,7 @@ bool AsmParser::parseDirectiveRept(SMLoc DirectiveLoc, StringRef Dir) {
53445347
return true;
53455348

53465349
int64_t Count;
5347-
if (!CountExpr->evaluateAsAbsolute(Count)) {
5350+
if (!CountExpr->evaluateAsAbsolute(Count, getStreamer().getAssemblerPtr())) {
53485351
return Error(CountLoc, "unexpected token in '" + Dir + "' directive");
53495352
}
53505353

‎llvm/lib/MC/MCStreamer.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ void MCTargetStreamer::emitValue(const MCExpr *Value) {
7575
void MCTargetStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {}
7676

7777
MCStreamer::MCStreamer(MCContext &Ctx)
78-
: Context(Ctx), CurrentWinFrameInfo(nullptr) {
78+
: Context(Ctx), CurrentWinFrameInfo(nullptr),
79+
UseAssemblerInfoForParsing(false) {
7980
SectionStack.push_back(std::pair<MCSectionSubPair, MCSectionSubPair>());
8081
}
8182

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# RUN: not llvm-mc -triple x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=ERR
2+
# RUN: not llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=ERR
3+
4+
.text
5+
6+
test2:
7+
jmp baz
8+
# ERR: [[@LINE+1]]:5: error: expected absolute expression
9+
.if . - text2 == 1
10+
nop
11+
.else
12+
ret
13+
.endif
14+
push fs
15+
16+
# No additional errors.
17+
#
18+
# ERR-NOT: {{[0-9]+}}:{{[0-9]+}}: error:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
; RUN: not llc -mtriple x86_64-unknown-linux-gnu -o %t.s -filetype=asm %s 2>&1 | FileCheck %s
2+
; RUN: not llc -mtriple x86_64-unknown-linux-gnu -o %t.o -filetype=obj %s 2>&1 | FileCheck %s
3+
4+
; Assembler-aware expression evaluation should be disabled in inline
5+
; assembly to prevent differences in behavior between object and
6+
; assembly output.
7+
8+
9+
; CHECK: <inline asm>:1:17: error: expected absolute expression
10+
11+
define i32 @main() local_unnamed_addr {
12+
tail call void asm sideeffect "foo: nop; .if . - foo==1; nop;.endif", "~{dirflag},~{fpsr},~{flags}"()
13+
ret i32 0
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# RUN: not llvm-mc -triple x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=ASM-ERR
2+
# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s | llvm-objdump -j .data -s - | FileCheck %s --check-prefix=OBJDATA
3+
# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s | llvm-objdump -j .text -s - | FileCheck %s --check-prefix=OBJTEXT
4+
.data
5+
6+
# OBJDATA: Contents of section .data
7+
# OBJDATA-NEXT: 0000 aa0506ff
8+
9+
foo2:
10+
# ASM-ERR: [[@LINE+1]]:5: error: expected absolute expression
11+
.if . - foo2 == 0
12+
.byte 0xaa
13+
.else
14+
.byte 0x00
15+
.endif
16+
17+
foo3:
18+
.byte 5
19+
# ASM-ERR: [[@LINE+1]]:5: error: expected absolute expression
20+
.if . - foo3 == 1
21+
.byte 6
22+
.else
23+
.byte 7
24+
.endif
25+
26+
.byte 0xff
27+
28+
# nop is a fixed size instruction so this should pass.
29+
30+
# OBJTEXT: Contents of section .text
31+
# OBJTEXT-NEXT: 0000 9090ff34 25
32+
33+
.text
34+
35+
text1:
36+
nop
37+
# ASM-ERR: [[@LINE+1]]:5: error: expected absolute expression
38+
.if . - text1 == 1
39+
nop
40+
.else
41+
ret
42+
.endif
43+
push gs
44+
45+
# No additional errors.
46+
#
47+
# ASM-ERR-NOT: {{[0-9]+}}:{{[0-9]+}}: error:

‎llvm/test/MC/AsmParser/directive_fill.s

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# RUN: llvm-mc -triple i386-unknown-unknown %s 2> %t.err | FileCheck %s
22
# RUN: FileCheck --check-prefix=CHECK-WARNINGS %s < %t.err
3-
# RUN: llvm-mc -triple i386-unknown-unknown -filetype=obj -o %t.o %s 2> %t.err
4-
# RUN: FileCheck --check-prefix=OBJ-WARNINGS %s < %t.err
3+
# RUN: llvm-mc -triple i386-unknown-unknown -filetype=obj -o %t.o %s 2> %t.err2
4+
# RUN: FileCheck --check-prefix=OBJ-WARNINGS %s < %t.err2
55

66
# CHECK: TEST0:
77
# CHECK: .fill 1, 1, 0xa

‎llvm/tools/llvm-mc/llvm-mc.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,9 @@ int main(int argc, char **argv) {
475475
Str->InitSections(true);
476476
}
477477

478+
// Use Assembler information for parsing.
479+
Str->setUseAssemblerInfoForParsing(true);
480+
478481
int Res = 1;
479482
bool disassemble = false;
480483
switch (Action) {

0 commit comments

Comments
 (0)
Please sign in to comment.