This is an archive of the discontinued LLVM Phabricator instance.

Use ".arch_extension" ARM directive to support hwdiv on krait
ClosedPublic

Authored by sgundapa on Feb 22 2015, 12:36 PM.

Details

Reviewers
rengolin
echristo
Summary

In case of "krait" CPU, asm printer doesn't emit any ".cpu" so the
features bits are not computed. This patch lets the asm printer
emit ".cpu cortex-a9" directive for krait and the hwdiv feature is
enabled through ".arch_extension". In short, krait is treated
as "cortex-a9" with hwdiv. We can not emit ".krait" as CPU since
it is not supported bu GNU GAS yet.

Diff Detail

Event Timeline

sgundapa updated this revision to Diff 20482.Feb 22 2015, 12:36 PM
sgundapa retitled this revision from to Use ".arch_extension" ARM directive to support hwdiv .
sgundapa updated this object.
sgundapa edited the test plan for this revision. (Show Details)
sgundapa added reviewers: rengolin, echristo.

The .arch_extension implementation patch is here: http://reviews.llvm.org/D7316

sgundapa added a subscriber: Unknown Object (MLST).Feb 24 2015, 11:48 AM
rengolin added inline comments.Feb 24 2015, 1:46 PM
lib/Target/ARM/ARMAsmPrinter.cpp
709

I was looking around and emitting plain text of a hardcoded value here would be a bad idea.

I think you need to create/append an enum and do like FPU/CPU/ARCH.

sgundapa added inline comments.Feb 24 2015, 6:42 PM
lib/Target/ARM/ARMAsmPrinter.cpp
709

Renato, Can you be more specifc on this ?
If you can point me to an existing implementation, it would be nice.

rengolin added inline comments.Feb 25 2015, 9:26 AM
lib/Target/ARM/ARMAsmPrinter.cpp
709

see emitFPU().

Valid values for name are the same as those accepted as architectural extensions by the -mcpu commandline option.

The same duplication that emitFPU has introduced (by adding the ARMFPUName.def) will need to be introduced, and that will all go away once we unify the parsers. For now, let's have it in the same way, so I can wipe them out all at once later.

sgundapa updated this revision to Diff 20717.Feb 25 2015, 4:38 PM

Hopefully, this addresses everyone's concerns

sgundapa retitled this revision from Use ".arch_extension" ARM directive to support hwdiv to Use ".arch_extension" ARM directive to support hwdiv on krait .Feb 25 2015, 5:11 PM
rengolin accepted this revision.Feb 26 2015, 8:09 AM
rengolin edited edge metadata.

With the comment inline sorted, LGTM, thanks!

lib/Target/ARM/ARMAsmPrinter.cpp
707

Can you move this up with the first check? I think it'll be easier to "remove Krait" later, as the FIXME hints to.

This revision is now accepted and ready to land.Feb 26 2015, 8:09 AM
sgundapa added inline comments.Feb 26 2015, 8:17 AM
lib/Target/ARM/ARMAsmPrinter.cpp
707

Sure. I will do that and commit this patch.
Thanks for your patience.

echristo accepted this revision.Feb 26 2015, 9:18 AM
echristo edited edge metadata.

Sure.

-eric

sgundapa closed this revision.Feb 27 2015, 8:52 AM

Thanks for the fix Renato