Advertisement

Close

I think this change make sense. I wouldn't worry about the AAPCS64 compliance as long as the (implicit, ill-defined) "LLVM IR PCS" is flexible enough, so as to allow implementing AAPCS64.
Perhaps it's useful to have more tests, on both sides of the calls and returns:

In D103167#2802486, @yroux wrote:

Advertisement

Close

@efriedma, @chill is this patch ok for you ? (it'd be great to have the issue fixed into 12.0.1 release)

Advertisement

Close

IMHO, it's possible to write a frontend test, which includes, say arm_neon.h, but does not really require the ARM or AArch64 backends to be configured (e.g. CodeGen/arm-vector-align.c?)
If arm_neon.h is not built, then the test would need the appropriate REQUIRES line, but than that means the frontend test coverage would decrease for people, who
are not interested in Arm backends. Mind you, even if a test is Arm specific, that does not mean it does not depend on generic code.

Updated a test to run for x86_64-linux instead of %itanium_abi_triple, to avoid having invalid
syntax for MACH-O sections. The patch itself does not care about section attribute syntax and x86 target
does not even need to be compiled.

Thanks, I'll have a look.

Moved a test from here to there, and did a few test tweaks, NFC.

LGTM, cheers!

Once test accidentally copied to the wrong directory, I'm going to fix that.

In D100853#2745376, @rnk wrote:

lgtm

Sorry about the delay.

Thanks!

Ping?

Rebase/retested on top of main

Apologies if you're already aware.

Fixed a merge conflict with https://reviews.llvm.org/D100467

Updated with only whitespace changes.

In D99098#2686627, @gchatelet wrote:

Thank you for the clean up. Really appreciate it.
Before submitting the patch did you check that there are no users in experimental targets? (ARC, VE)

In D98794#2673766, @chill wrote:

This needs to be changed, cf. https://lists.llvm.org/pipermail/llvm-dev/2021-March/149405.html

I did minor test updates, following reviewers' suggestions, don't think another review is warranted, going to commit.

In D99642#2660895, @nikic wrote:

I think this should be using Val->stripInBoundsOffsets() rather than modify the getUnderlyingObject() API.

This needs to be changed, cf. https://lists.llvm.org/pipermail/llvm-dev/2021-March/149405.html

In D94083#2486800, @fhahn wrote:

FWIW I think it would be good to have a bit more details in the description for changes such as this, like a link to the public docs for the extension.

LGTM

LGTM.

FYI: https://bugs.llvm.org/show_bug.cgi?id=48472

In D91994#2422228, @chill wrote:

In the Arm Architecture Reference Manual Supplement - Armv8, for Armv8-R AArch64 architecture profile
"A1.4 Architecture extensions" lists among others

• FEAT_SHA1 Advanced SIMD SHA1 instructions
• FEAT_SHA256 Advanced SIMD SHA256 instructions
• FEAT_AES Advanced SIMD AES instructions
• FEAT_PMULL Advanced SIMD PMULL instructions

Is there a document, which says R82 does not implement those ?

In the Arm Architecture Reference Manual Supplement - Armv8, for Armv8-R AArch64 architecture profile
"A1.4 Architecture extensions" lists among others

LGTM, thanks.

Thanks!

Cheers!

Cheers!

That should be the minimum number of instructions now, one less and the outlining does not pass the cost threshold.

Ping.

In D89625#2367785, @yroux wrote:

I'd check if there are some calls of modifiesRegister that pass nullptr, the patch LGTM otherwise

Ping.

Ping.

Ping.

Ping.