arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.
LGTM. with nits The lower could handle the vector case easily, but it didn't
before either
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2915
+SDValue Op =
Pierre-vh added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening,
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening, some
Pierre-vh added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5573-5576
+ SDLoc SL(Op);
+ return DAG.getNode(
+ ISD::FP_EXTEND, SL, MVT::f32,
+ DAG.getNode(ISD::BITCAST, SL, MVT::f16, Op->getOperand(0)));
arsenm wrote:
>
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5573-5576
+ SDLoc SL(Op);
+ return DAG.getNode(
+ ISD::FP_EXTEND, SL, MVT::f32,
+ DAG.getNode(ISD::BITCAST, SL, MVT::f16, Op->getOperand(0)));
arsenm wrote:
>
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:913
+ else
+RegisterVT = (ScalarVT == MVT::bf16 ? MVT::v2bf16 : MVT::v2f16);
IntermediateVT = RegisterVT;
Pierre-vh wrote:
> arsenm wrote:
> > If you wanted
Pierre-vh added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:913
+ else
+RegisterVT = (ScalarVT == MVT::bf16 ? MVT::v2bf16 : MVT::v2f16);
IntermediateVT = RegisterVT;
arsenm wrote:
> If you wanted the promote to i32,
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCallingConv.td:49
+ CCIfType<[bf16], CCBitConvertToType>,
+ CCIfType<[v2bf16], CCBitConvertToType>,
CCIfNotInReg arsenm wrote:
> > Without being added to a register class, all the tablegen changes should
Pierre-vh added inline comments.
Comment at: clang/lib/Basic/Targets/AMDGPU.h:119
+ bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); }
+ const char *getBFloat16Mangling() const override { return "u6__bf16"; };
+
arsenm wrote:
> Pierre-vh
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCallingConv.td:49
+ CCIfType<[bf16], CCBitConvertToType>,
+ CCIfType<[v2bf16], CCBitConvertToType>,
CCIfNotInReghttps://reviews.llvm.org/D139398/new/
https://reviews.llvm.org/D139398
arsenm added inline comments.
Comment at: clang/lib/Basic/Targets/AMDGPU.h:119
+ bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); }
+ const char *getBFloat16Mangling() const override { return "u6__bf16"; };
+
Pierre-vh wrote:
> Pierre-vh
Pierre-vh added inline comments.
Comment at: clang/lib/Basic/Targets/AMDGPU.h:119
+ bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); }
+ const char *getBFloat16Mangling() const override { return "u6__bf16"; };
+
Pierre-vh wrote:
> arsenm
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening, some
Pierre-vh added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening,
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening, some
Pierre-vh added inline comments.
Comment at: clang/lib/Basic/Targets/AMDGPU.h:119
+ bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); }
+ const char *getBFloat16Mangling() const override { return "u6__bf16"; };
+
arsenm wrote:
> Don't
arsenm added inline comments.
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:147
addRegisterClass(MVT::f16, ::SReg_32RegClass);
+addRegisterClass(MVT::bf16, ::SReg_32RegClass);
Do you really need to add this to a register class? The only thing
arsenm requested changes to this revision.
arsenm added inline comments.
This revision now requires changes to proceed.
Comment at: clang/lib/Basic/Targets/AMDGPU.h:119
+ bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); }
+ const char
Pierre-vh updated this revision to Diff 480431.
Pierre-vh added a comment.
- Only accept bf16 on AMDGCN; r600 doesn't support it (we could but it's not
worth the effort I think; I'll look at it if we find out it's needed)
- Remove bf16 types from a few register classes
Repository:
rG LLVM
Pierre-vh created this revision.
Pierre-vh added reviewers: arsenm, foad, yaxunl.
Herald added subscribers: kosarev, kerbowa, hiraditya, tpr, dstuttard, jvesely,
kzhuravl.
Herald added a project: All.
Pierre-vh requested review of this revision.
Herald added subscribers: llvm-commits,
20 matches
Mail list logo
