This is an archive of the discontinued LLVM Phabricator instance.

[mips][ias] Handle more complicated expressions for memory operands
ClosedPublic

Authored by sdardis on Sep 16 2016, 7:47 AM.

Details

Summary

This patch teaches ias for mips to handle expressions such as
(8*4)+(8*31)($sp). Such expression typically occur from the expansion
of multiple macro definitions.

This partially resolves PR/30383.

Diff Detail

Repository
rL LLVM

Event Timeline

sdardis updated this revision to Diff 71638.Sep 16 2016, 7:47 AM
sdardis retitled this revision from to [mips][ias] Handle more complicated expressions for memory operands.
sdardis updated this object.
sdardis added subscribers: llvm-commits, seanbruno, emaste.
sdardis updated this revision to Diff 71652.Sep 16 2016, 8:11 AM

Whitespace, variable name changed.

vkalintiris requested changes to this revision.Sep 22 2016, 8:20 AM
vkalintiris edited edge metadata.
vkalintiris added inline comments.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
4447–4453 ↗(On Diff #71652)

We should test the comparison/equality operators in MC/Mips/memory-offsets.s as well. Can you check that we follow the GNU assembler behaviour for these?

This revision now requires changes to proceed.Sep 22 2016, 8:20 AM
sdardis added inline comments.Oct 11 2016, 9:18 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
4447–4453 ↗(On Diff #71652)

I remember why I didn't test them in memory-offsets.s now. IAS will resolve the result of the comparison to '1' or '0'. GAS behaves oddly and gives 0 or -1. Given the differences, I think I should drop them altogether as I'm struggling to comprehend why someone would use a relational operator in a memory offset.

sdardis updated this revision to Diff 74367.Oct 12 2016, 5:58 AM
sdardis edited edge metadata.

Removed comparison operator handling.

vkalintiris accepted this revision.Oct 18 2016, 6:36 AM
vkalintiris edited edge metadata.

LGTM.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
4447–4453 ↗(On Diff #71652)

I think we should drop them too. Maybe we should add a comment at the top of the switch statement to remember the difference.

This revision is now accepted and ready to land.Oct 18 2016, 6:36 AM
This revision was automatically updated to reflect the committed changes.

Thanks for the review.