[PATCH] D91927: [X86] Add x86_amx type for intel AMX.
LuoYuanke via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 22 15:21:00 PST 2020
LuoYuanke added inline comments.
================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:264
+ SmallVector<Instruction *, 8> DeadInsts;
+ SmallVector<Instruction *, 2> DeadBitcasts;
+
----------------
pengfei wrote:
> Maybe better to use BitCastInst?
There may be dead load or store instructions.
================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:280
+ Type *Ty = Bitcast->getType();
+ auto CanonicalizeBitcast = [&]() {
+ bool Change = false;
----------------
pengfei wrote:
> Can we leave the canonicalize bitcast cases a single patch. It's a bit complex here and I don't think it's a common case.
Ok, I'll create another patch for it.
================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:193
+ // %2 = load <256 x i32>, <256 x i32>* %addr, align 1024
+ auto *II = cast<IntrinsicInst>(Src);
+ Value *Row = II->getOperand(0);
----------------
pengfei wrote:
> Is it possible the x86_amx operand isn't from AMX intrinsic, e.g.
> ```
> %src = bitcast <256 x i32> %xxx to x86_amx
> %2 = bitcast x86_amx %src to <256 x i32>
> ```
Good catch. I'll add support for this pattern.
================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:238
for (Instruction &Inst : BB) {
- LoadInst *LD = dyn_cast<LoadInst>(&Inst);
- // Check load instruction.
- // %3 = load <256 x i32>, <256 x i32>* %1, align 64
- if (LD) {
- FixedVectorType *VTy = dyn_cast<FixedVectorType>(Inst.getType());
- if (!IsAMXType(VTy))
- continue;
- LDSet.insert(&Inst);
+ if (!dyn_cast<BitCastInst>(&Inst))
continue;
----------------
pengfei wrote:
> Better to reuse the cast result, e.g.
> ```
> BitCastInst *BInst = dyn_cast<BitCastInst>(&Inst);
> if (!BInst )
> ```
> You can save several `cast<BitCastInst>(&Inst)` below.
That's good. Thanks.
================
Comment at: llvm/test/CodeGen/X86/AMX/amx-across-func.ll:89-91
attributes #2 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="8192" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+amx-int8,+amx-tile,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #3 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+amx-int8,+amx-tile,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #4 = { nounwind }
----------------
pengfei wrote:
> Better to remove these unused attributes. The same to other tests.
I'll create a separate patch to clean the attributes.
================
Comment at: llvm/test/CodeGen/X86/AMX/amx-type.ll:67
+
+define dso_local void @test_src_add(<256 x i32> %x, <256 x i32> %y, i16 %r, i16 %c, i8* %buf, i64 %s) #2 {
+; CHECK-LABEL: @test_src_add(
----------------
pengfei wrote:
> For this and the next test, we have chances to optimize to memcpy if we can make sure %s is constant 64.
If the stride is 64 we can transform the code to memcpy. How about do it in another patch?
================
Comment at: llvm/test/CodeGen/X86/AMX/amx-type.ll:103
+
define dso_local void @test_load(i8* %in, i8* %out) local_unnamed_addr #2 {
; CHECK-LABEL: @test_load(
----------------
pengfei wrote:
> We don't need to check this case now, right?
It can check the load and store instruction is not transformed if they are not participate in amx operation. I prefer to keep the case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91927/new/
https://reviews.llvm.org/D91927
More information about the llvm-commits
mailing list