Page MenuHomePhabricator

[llc+llvm-mc] Replace the hidden -target-abi option with a -mabi thats visible in --help.
Needs ReviewPublic

Authored by sdardis on Jun 17 2016, 4:06 AM.

Details

Summary

Although this patch is a cosmetic change, it is important for MIPS since it
is a prerequisite of moving the ABI to the environment component of the
triple (much like X86's and ARM's GNUX32, GNUEABI, and GNUEABIHF) which in
turn is important for both enabling IAS by default for the N64 ABI and for
distinguishing between the N32 and N64 in order to fix IAS for N32 and
prevent inappropriate interlinking in the IRLinker. Follow-up patches will try
to modify the triple according to -mabi for targets that prefer this instead of
passing it to MCTargetOptions::ABIName.

See http://reviews.llvm.org/D20916 (which this patch has been extracted
from) for more detail on the wider context of this patch as well as an
explanation as to why having the ABI name in MCTargetOptions does not work
for Mips.

The test changes are all switching -target-abi to -mabi.

A similar patch will be posted for 'clang -cc1' and 'clang -cc1as' (the
latter currently accepts a -target-abi option but ignores it, I'll fix this
at the same time).

Patch By: Daniel Sanders

Event Timeline

dsanders updated this revision to Diff 61079.Jun 17 2016, 4:06 AM
dsanders retitled this revision from to [llc+llvm-mc] Replace the hidden -target-abi option with a -mabi thats visible in --help..
dsanders updated this object.
dsanders added a subscriber: llvm-commits.

If it will be in the triple, why do you need the -mabi command line option?

dsanders updated this revision to Diff 61778.Jun 24 2016, 5:25 AM

Refresh patch after following the commit of r273557.

dsanders updated this revision to Diff 63489.Jul 11 2016, 5:15 AM

Refresh patch. This patch is unaffected by the recent discussion on llvm-dev.

Given that we agreed on using triples to differentiate n32 and n64, is this
worth it? A followup patch would just delete the option and use the triple,
no?

I think it's worth it. Most backend tests currently mutate the default target triple (often the host triple) and converting all of those to fully specify a target triple using –triple/-mtriple is a big task that isn't a pre-requisite of fixing the N32 vs N64 problem.

From: Rafael Espíndola [mailto:rafael.espindola@gmail.com]
Sent: 11 July 2016 16:52
To: reviews+D21465+public+92eb7897bc2cfded@reviews.llvm.org
Cc: Daniel Sanders; nemanja.i.ibm@gmail.com; amara.emerson@arm.com; tberghammer@google.com; danalbert@google.com; srhines@google.com; llvm-commits@lists.llvm.org; t.p.northover@gmail.com; filcab+llvm.phabricator@gmail.com
Subject: Re: [PATCH] D21465: [llc+llvm-mc] Replace the hidden -target-abi option with a -mabi thats visible in --help.

Given that we agreed on using triples to differentiate n32 and n64, is this worth it? A followup patch would just delete the option and use the triple, no?

dsanders updated this revision to Diff 64858.Jul 21 2016, 4:55 AM

Refresh following recent test additions.

Rafael: Are you ok with this patch as-is and coming back to eliminating

-mabi/-target-abi in later patches or do you feel strongly that
this should be done up front.
rafael removed a subscriber: rafael.
dsanders updated this revision to Diff 65902.Jul 28 2016, 3:42 AM

Refresh to account for newly-added tests and ping

dsanders updated this revision to Diff 65926.Jul 28 2016, 6:52 AM

Refresh and ping

One last ping since I need to either commit this series next week or hand over to a colleague to continue it

sdardis commandeered this revision.Aug 23 2016, 6:19 AM
sdardis added a reviewer: dsanders.
sdardis added a subscriber: sdardis.

Taking over this patch series.

sdardis updated this revision to Diff 68979.Aug 23 2016, 6:20 AM
sdardis edited edge metadata.

Rebase + ping. I am aware that you're exploring an alternate solution Eric.

sdardis updated this object.Aug 23 2016, 6:21 AM
sdardis updated this revision to Diff 73049.Sep 30 2016, 7:38 AM

Refresh and ping.

sdardis updated this revision to Diff 76723.Nov 2 2016, 9:55 AM
sdardis edited edge metadata.

Rebase and ping.

Hello,

Are there plans to merge this in? I have updated the patch onto 0d7672e (attached), if it is helpful, though it was mostly just s/target-abi[ =]/mabi=/ on the files which failed the tests (and then ran the tests again and nothing failed).

Thanks,
Robert

dsanders edited edge metadata.Jul 19 2017, 3:55 AM

Hello,

Are there plans to merge this in? I have updated the patch onto 0d7672e (attached), if it is helpful, though it was mostly just s/target-abi[ =]/mabi=/ on the files which failed the tests (and then ran the tests again and nothing failed).

Thanks,
Robert

Hi,

I would love to see this committed along with the rest of the work to fix the conflict between LLVM's triples and the triples used by various distros and cross-compilers. However, I don't think I can justify working on it as the targets I work on nowadays are consistent with the way LLVM expects triples to work.

dsanders resigned from this revision.Jul 18 2019, 6:56 PM