Page MenuHomePhabricator

[AArch64] Add support for 256-bit non temporal loads
ClosedPublic

Authored by zjaffal on Aug 12 2022, 6:34 AM.

Details

Summary

Currenlty all temporal loads are mapped to LDP or LDR. This patch will map all the non temporal 256-bit loads into LDNP. Future patches should address other non-temporal loads.

Diff Detail

Unit TestsFailed

TimeTest
2,140 msx64 debian > LLVM.CodeGen/AArch64::aarch64-addv.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/aarch64-addv.ll -mtriple=aarch64-eabi -aarch64-neon-syntax=generic | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/aarch64-addv.ll
1,980 msx64 debian > LLVM.CodeGen/AArch64::aarch64-minmaxv.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll -mtriple=aarch64-linux--gnu -aarch64-neon-syntax=generic | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll
2,190 msx64 debian > LLVM.CodeGen/AArch64::arm64-alloc-no-stack-realign.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/arm64-alloc-no-stack-realign.ll -mtriple=arm64-apple-darwin -enable-misched=false -enable-post-misched=false | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/arm64-alloc-no-stack-realign.ll
1,920 msx64 debian > LLVM.CodeGen/AArch64::arm64-convert-v4f64.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/arm64-convert-v4f64.ll -mtriple=arm64-eabi | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/arm64-convert-v4f64.ll
1,900 msx64 debian > LLVM.CodeGen/AArch64::arm64-fmuladd.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/arm64-fmuladd.ll -asm-verbose=false -mtriple=arm64-eabi -aarch64-neon-syntax=apple | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/arm64-fmuladd.ll
View Full Test Results (13 Failed)

Event Timeline

zjaffal created this revision.Aug 12 2022, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 6:34 AM
zjaffal requested review of this revision.Aug 12 2022, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 6:34 AM
fhahn added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
793

Could you add a comment similar to the one for the stores above?

804

nit: please add a newline before this comment.

20394

Would be good to have a comment here to describe what's lowered here.

20409

leftover commented-out code?

llvm/lib/Target/AArch64/AArch64InstrInfo.td
2588

nit: newline here

dmgreen added inline comments.Aug 13 2022, 3:25 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
20395

Why is EC.isKnownEven needed?

20396

This looks like it could drop a set of brackets.

zjaffal updated this revision to Diff 452422.Aug 13 2022, 7:07 AM
zjaffal marked an inline comment as done.

Remove even checks and add comments

zjaffal marked 5 inline comments as done.Aug 16 2022, 12:47 AM
zjaffal added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
20395

I followed the same code that is used to lower STNP but I think it is not really necessary here.
I think we can remove it.

dmgreen accepted this revision.Aug 16 2022, 1:44 AM

LGTM if @fhahn doesn't know of a reason to add the isKnownEven.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
20395

Yeah - I agree so long as it is checking the size of the MemVT and it's scalar element size.

This revision is now accepted and ready to land.Aug 16 2022, 1:44 AM
fhahn added inline comments.Aug 16 2022, 2:16 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
20413

I think this assertion doesn't really make sense any longer, because we now enable the custom lowering for additional types. This probably needs to became just a check (i.e add an early exit here if the cownciditon doesn't hold). At the moment this assertion triggers in multiple tests, causing them to fail.

fhahn added a comment.Aug 16 2022, 2:23 AM

LGTM if @fhahn doesn't know of a reason to add the isKnownEven.

I can't think of any reason why this should be needed now and also couldn't find anything when looking at the original review. @zjaffal it would be great if you could post a follow-up patch that removes isKnownEven from the checks for STNP or even better move the checks to a helper function used by both STNP and LDNP handling code.

zjaffal updated this revision to Diff 452947.Aug 16 2022, 4:11 AM
zjaffal marked an inline comment as done.

Fix for failing tests

fhahn accepted this revision.Aug 16 2022, 4:14 AM

LGTM, thanks!

zjaffal marked 2 inline comments as done.Aug 16 2022, 4:15 AM
This revision was landed with ongoing or failed builds.Aug 16 2022, 4:19 AM
This revision was automatically updated to reflect the committed changes.