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 wrote: > > Pierre-vh wrote: > > > arsenm wrote: > > > > Don't understand this mangling. What is u6? > > > Not sure; for that one I just copy-pasted the implementation of other > > > targets. All other targets use that mangling scheme > > Ah I remember now, it's just C++ mangling. I don't quite understand the > > lowercase "u" but a quick search in Clang tells me it's vendor-extended > > types. > > So it's just u6 -> vendor extended type, 6 characters following + __bf16 > > (name of the type). > Do we really need an override for this? I'd expect a reasonable default. Plus > I think a virtual function for something that's only a parameterless, static > string is a bit ridiculous Default impl asserts if not implemented. I think it's to make sure targets are all aware of what it takes to support bfloat and they don't end up partially implementing it? ``` /// Return the mangled code of bfloat. virtual const char *getBFloat16Mangling() const { llvm_unreachable("bfloat not implemented on this target"); } ``` I'd say let's stick to the current pattern in this diff; I created D139608 to change it ================ Comment at: llvm/lib/Target/AMDGPU/AMDGPUCallingConv.td:49 + CCIfType<[bf16], CCBitConvertToType<i16>>, + CCIfType<[v2bf16], CCBitConvertToType<i32>>, CCIfNotInReg<CCIfType<[f32, i32, f16, i16, v2i16, v2f16] , CCAssignToReg<[ ---------------- arsenm wrote: > Without being added to a register class, all the tablegen changes should not > do anything bf16 ones seem to not be needed but if I don't have the v2bf16 ones I get "cannot allocate arguments" in "test_arg_store_v2bf16" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139398/new/ https://reviews.llvm.org/D139398 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits