[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas added a comment. Hi @hans , I'll have a look at it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75169/new/ https://reviews.llvm.org/D75169 ___ cfe-commits mailing list

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

hans added a comment. Lucas, this seems to have casued https://bugs.llvm.org/show_bug.cgi?id=47001. Can you take a look? (I would cc you on the bug, but I couldn't find your email in bugzilla.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

plotfi added a comment. @pratlucas @ostannard @rjmccall I've posted a diff D82443 to address what I think could be a potential fix for the assert I was seeing on the provided reduced IR. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

plotfi added inline comments. Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:4106 +unsigned NumParts, MVT PartVT, Optional CC) const { + bool IsABIRegCopy = CC.hasValue(); + EVT ValueVT = Val.getValueType(); I'm not well versed in arm CCs, but I

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

plotfi added a comment. @pratlucas @ostannard @rjmccall I have noticed this change break on the building of the Swift standard library in downstream apple/swift/master-next. I reduced the test case and found that the chain of bitcasts/extends in ARMTargetLowering::splitValueIntoRegisterParts

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

This revision was automatically updated to reflect the committed changes. Closed by commit rGa255931c4055: [ARM] Supporting lowering of half-precision FP arguments and returns in… (authored by pratlucas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. I don't think it makes sense to make `f16` legal for targets which don't have any arithmetic operations on it, since that would be contrary to the definition of "legal". I'd also expect

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas added a comment. Hi @efriedma, From @SjoerdMeijer's comment and the links he pointed to, it seems to me that making `f16` types legal for all ARM subtargets would be a major undertaking and far from trivial to implement. It's also not clear to me how significant would be the returns

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

dnsampaio added a comment. Perhaps we could move to making half a valid type for the arm back-end as follow up patches. Allowing half as argument through the IR is already a step to that direction. IMO this patch is already quite big and it excels in fixing the bugs it proposed. Repository:

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

SjoerdMeijer added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); efriedma wrote: > pratlucas wrote: > > efriedma wrote: >

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas updated this revision to Diff 270143. pratlucas added a comment. Fixing failure on CodeGen/ARM/GlobalISel/arm-unsupported.ll and making clang-format happy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75169/new/

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); pratlucas wrote: > efriedma wrote: > > I'm not sure I

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas marked an inline comment as done. pratlucas added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); efriedma wrote:

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas updated this revision to Diff 269915. pratlucas marked 2 inline comments as done. pratlucas added a comment. Clean-ups + fixing failure in CodeGen/ARM/half.ll test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75169/new/

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); I'm not sure I understand why the standard

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

dmgreen added reviewers: efriedma, ostannard, SjoerdMeijer. dmgreen added a comment. Love it. This will help optimise these more too. We can currently get into places where we can't prove only the bottom bits of a returned value are demanded so can't remove unnecessary vmovs. Test look good too

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas updated this revision to Diff 269466. pratlucas added a comment. Formatting patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75169/new/ https://reviews.llvm.org/D75169 Files: llvm/include/llvm/CodeGen/TargetLowering.h

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

pratlucas updated this revision to Diff 269450. pratlucas added a comment. Splitting the patch into two parts: one for introducing the half-precision handling into AArch32's backend and one for removing the existing coercion of those arguments from Clang. Repository: rG LLVM Github Monorepo