Page MenuHomePhabricator

[X86][AMX] Prohibit pointer cast on load.
ClosedPublic

Authored by LuoYuanke on Jan 9 2021, 10:10 PM.

Details

Summary

The load/store instruction will be transformed to amx intrinsics in the
pass of AMX type lowering. Prohibiting the pointer cast make that pass
happy.

Diff Detail

Event Timeline

LuoYuanke created this revision.Jan 9 2021, 10:10 PM
LuoYuanke requested review of this revision.Jan 9 2021, 10:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2021, 10:10 PM
LuoYuanke updated this revision to Diff 315650.Jan 9 2021, 10:14 PM

Fix typo in the comments.

Harbormaster completed remote builds in B84603: Diff 315650.

to amx intrinsics the the

to amx intrinsics that?

llvm/test/Transforms/InstCombine/load.ll
427 ↗(On Diff #315650)

Missing check results?

LuoYuanke edited the summary of this revision. (Show Details)Jan 10 2021, 3:48 AM
LuoYuanke added inline comments.Jan 10 2021, 3:51 AM
llvm/test/Transforms/InstCombine/load.ll
427 ↗(On Diff #315650)

Oops. Thank Pengfei. :)

LuoYuanke updated this revision to Diff 315660.Jan 10 2021, 3:59 AM

Address Pengfei's comments.

pengfei added inline comments.Jan 10 2021, 4:42 AM
llvm/test/Transforms/InstCombine/load.ll
452 ↗(On Diff #315660)

I think we had disabled the chance that x86_amx* been generated by compiler. It may mean somthing wrong if we see such load. So I think it's better just raise an assert fail for it here.
Besides, it looks safe and better for performance to combine

%vec = load x86_amx, x86_amx* %src, align 64
%bc = bitcast x86_amx %vec to <256 x i32>

to

%bc = load <256 x i32>, <256 x i32>* %src, align 64
lebedev.ri requested changes to this revision.Jan 10 2021, 4:59 AM
lebedev.ri added a subscriber: lebedev.ri.
This comment was removed by lebedev.ri.
This revision now requires changes to proceed.Jan 10 2021, 4:59 AM

What specifically is having problems after that transformation?

This revision now requires review to proceed.Jan 10 2021, 5:01 AM

What specifically is having problems after that transformation?

The the amx load require shape and stride. We need transform "%vec = load x86_amx, x86_amx* %src" to "%vec = @llvm.x86.tileloadd64.internal(i16 %row, i16 %col, i8* %src, i64 %stride)". The %row, %col need to be deduced from the context. Prohibit the transform make pass "lower-amx-type" happy.

LuoYuanke added inline comments.Jan 10 2021, 5:43 AM
llvm/test/Transforms/InstCombine/load.ll
452 ↗(On Diff #315660)

So we may delete this case, as we think "x86_amx*" is invalid. I can take x86_amx* as invalid in IR verification in another patch.

LuoYuanke updated this revision to Diff 315981.Jan 11 2021, 7:43 PM

Address Pengfei's comments.

pengfei accepted this revision.Jan 11 2021, 8:41 PM

LGTM, but I'd like to see if @lebedev.ri has any objections.

This revision is now accepted and ready to land.Jan 11 2021, 8:41 PM

LGTM, but I'd like to see if @lebedev.ri has any objections.

Ok, I'll wait 1 day to see if @lebedev.ri has any objection for the patch.

This revision was automatically updated to reflect the committed changes.