This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen][AArch64] Fix isel crash for truncating FP stores
ClosedPublic

Authored by david-arm on Apr 7 2021, 1:52 AM.

Details

Summary

When attempting to truncate a FP vector and store the result out
to memory we crashed because we had no pattern for truncating FP
stores. In fact, we don't support these types of stores and the
correct fix is to stop marking these truncating stores as legal.

Tests have been added here:

CodeGen/AArch64/sve-fptrunc-store.ll

Diff Detail

Event Timeline

david-arm created this revision.Apr 7 2021, 1:52 AM
david-arm requested review of this revision.Apr 7 2021, 1:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2021, 1:52 AM
joechrisellis accepted this revision.Apr 7 2021, 2:03 AM

LGTM. 😄

This revision is now accepted and ready to land.Apr 7 2021, 2:03 AM
sdesmalen added inline comments.Apr 7 2021, 6:50 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1183

nit: could you leave a comment explaining why this is set to Expand?

llvm/test/CodeGen/AArch64/sve-fptrunc-store.ll
3

This check for a warning is no longer necessary since D98856

david-arm added inline comments.Apr 7 2021, 6:56 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1183

Sure, there is no comment for the Neon case that I can borrow from, but I can write something like this if that seems reasonable?

// Avoid marking truncating FP stores as legal to prevent the DAGCombiner from creating
// unsupported truncating stores.
david-arm updated this revision to Diff 335819.Apr 7 2021, 8:28 AM
  • Added a comment and removed warnings check from test.
sdesmalen accepted this revision.Apr 7 2021, 8:35 AM