[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