[PATCH] D45205: [ARM] FP16 VSEL codegen follow up
Sjoerd Meijer via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 4 02:21:43 PDT 2018
SjoerdMeijer added a comment.
Ah, yes, should have explained that a bit better. This is related to your earlier comment about:
> Does this need to check Subtarget->hasFullFP16()?
Instead of doing the f16 and hasFullFP16() check inside function LowerSELECT_CC, I have guarded custom lowering of the Node with the hasFullFP16 check. While doing this, I spotted the custom lowering of SETCC, SELECT, SELECT_CC and BR_CC for f16, and I think it is more correct to guard these custom lowerings with hasFullFP16 as well; that's what we should be doing. Thus, this is indeed a non-functional change. And they should already be regression tested. BR_CC is for example added in https://reviews.llvm.org/D43508 and has tests, and file fp16-instructions.ll should test the others too.
https://reviews.llvm.org/D45205
More information about the llvm-commits
mailing list