[PATCH] D96283: [DAGCombine] Do not remove masking argument to FP16_TO_FP for some targets

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 9 03:07:47 PST 2021


nemanjai added a comment.

In D96283#2550702 <https://reviews.llvm.org/D96283#2550702>, @t.p.northover wrote:

> We already seem to have `shouldSignExtendTypeInLibCall` (used in the actual `ExpandLibCall` this code gets to) and `shouldExtendTypeInLibCall` (used in another expander I didn't know about). Ideally that's where we'd fix this issue by getting the generic call lowerer to insert a zext (which may be folded again later, if it's a sound transformation). Unfortunately because we only have an `i32` when the expansion happens I don't think that's possible.

My initial thought was along these lines. In fact, I tried to do this in the PPC call lowering but no information exists at that point that this should be a 16-bit value. This would kind of require that I use the name of the function or something similarly flaky which I don't find acceptable. Ultimately, the combine is quite specific (the masking of arguments to normal calls is not removed) so the fix had to be quite specific.

> But this seems like a much more targetted fix that only applies to this one f16 conversion so the name should probably reflect that.

Updated the name. My initial thought was to make it a somewhat generic name in case this is useful for other similar transformations in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96283/new/

https://reviews.llvm.org/D96283



More information about the llvm-commits mailing list