This is an archive of the discontinued LLVM Phabricator instance.

Differential D20656

[MC] Support symbolic expressions in assembly directives
ClosedPublic

Authored by phosek on May 25 2016, 5:22 PM.

Details

Reviewers
• rafael
Commits
rGec73d8b383a1: [MC] Support symbolic expressions in assembly directives
rL271028: [MC] Support symbolic expressions in assembly directives
Summary

This matches the behavior of GNU assembler which supports symbolic
expressions in absolute expressions used in assembly directives.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek updated this revision to Diff 58543.May 25 2016, 5:22 PM
phosek retitled this revision from to [MC] Remove the leftover warning check.
phosek updated this object.
phosek added a reviewer: • rafael.
phosek set the repository for this revision to rL LLVM.
phosek added a comment.May 25 2016, 5:26 PM

This should fix the error I introduced in the previous change.

phosek added a subscriber: llvm-commits.May 25 2016, 5:34 PM
phosek added a comment.May 25 2016, 5:38 PM

This should test failure I introduced in D8072, if someone can review the change, I'd be happy to land this to unbreak the tree.

• rafael edited edge metadata.May 26 2016, 4:50 AM

How about using -filetype=obj? You should still get a warning with that, no?

phosek updated this revision to Diff 58678.May 26 2016, 1:12 PM
phosek retitled this revision from [MC] Remove the leftover warning check to [MC] Use obj output for .fill tests where needed.
phosek updated this object.
phosek edited edge metadata.

That's a better solution.

• rafael added a subscriber: • rafael.May 26 2016, 6:21 PM

LGTM

phosek added a comment.May 26 2016, 8:23 PM

Shall I mark this as accepted?

phosek updated this revision to Diff 58756.EditedMay 26 2016, 10:53 PM
phosek retitled this revision from [MC] Use obj output for .fill tests where needed to [MC] Support symbolic expressions in assembly directives.
phosek updated this object.

The original change was reverted because of the test failure, so I've updated this revision to include both the original change and the updated test which is now passing.

• rafael added a comment.May 27 2016, 3:22 AM

Up to you. Using phab is optional, so just a lgtm in the list is sufficient.

• rafael added a comment.May 27 2016, 3:28 AM

Lgtm. Just be sure to run check-all before committing :-)

Closed by commit rL271028: [MC] Support symbolic expressions in assembly directives (authored by phosek). · Explain WhyMay 27 2016, 11:56 AM
This revision was automatically updated to reflect the committed changes.
phosek added a comment.May 27 2016, 12:42 PM

This is strange, I did run all the test several times on my machine (Ubuntu x86-64) and all tests passed, but after landing the change, I got several failures on different platforms such as OS X . The source of the failure is unclear to me as the expression emitted for .fill 1, 1, 10 is .fill 1, 1, 0x0 instead of .fill 1, 1, 0xa. I'm not sure if it's related to the platform, but I'll get an OS X machine and try to replicate the failure.

phosek added a comment.May 27 2016, 12:43 PM

I should have also mentioned that I've reverted the change myself to avoid further build breakages, sorry about all the issues related to this change.

phosek mentioned this in rL271038: Revert "[MC] Support symbolic expressions in assembly directives".May 27 2016, 1:08 PM
phosek mentioned this in D20752: [MC] Support symbolic expressions in assembly directives.May 27 2016, 2:38 PM

Revision Contents

PathSize
test/
MC/
AsmParser/
9 lines

Diff 58678

test/MC/AsmParser/directive_fill.s

# RUN: llvm-mc -triple i386-unknown-unknown %s 2> %t.err | FileCheck %s # RUN: llvm-mc -triple i386-unknown-unknown %s 2> %t.err | FileCheck %s
# RUN: FileCheck --check-prefix=CHECK-WARNINGS %s < %t.err # RUN: FileCheck --check-prefix=CHECK-WARNINGS %s < %t.err
# RUN: llvm-mc -triple i386-unknown-unknown -filetype=obj -o %t.o %s 2> %t.err
# RUN: FileCheck --check-prefix=OBJ-WARNINGS %s < %t.err
# CHECK: TEST0: # CHECK: TEST0:
# CHECK: .fill 1, 1, 0xa # CHECK: .fill 1, 1, 0xa
Context not available.
TEST7: TEST7:
.fill 1, 8, 1<<32 .fill 1, 8, 1<<32
# CHECK-WARNINGS: '.fill' directive with negative repeat count has no effect # CHECK: TEST8
# CHECK: .fill -1, 8, 0x1
# OBJ-WARNINGS: '.fill' directive with negative repeat count has no effect
TEST8: TEST8:
.fill -1, 8, 1 .fill -1, 8, 1
Context not available.
TEST10: TEST10:
.fill 1, 3, 0x12345678 .fill 1, 3, 0x12345678
# CHECK: TEST11
# CHECK: .fill TEST11-TEST10, 1, 0x0 # CHECK: .fill TEST11-TEST10, 1, 0x0
TEST11: TEST11:
.fill TEST11 - TEST10 .fill TEST11 - TEST10
# CHECK: TEST12
# CHECK: .fill TEST11-TEST12, 3, 0x12345678 # CHECK: .fill TEST11-TEST12, 3, 0x12345678
# OBJ-WARNINGS: '.fill' directive with negative repeat count has no effect
TEST12: TEST12:
.fill TEST11 - TEST12, 3, 0x12345678 .fill TEST11 - TEST12, 3, 0x12345678
Context not available.