[PATCH] D15331: PR25754: implement result legalization for UDIVREM8_ZEXT_HREG

[Sanjay Patel via llvm-commits]

spatel added reviewers: andreadb, qcolombet.
spatel added a comment.

Thanks for working on this!

This patch solves the bug, but I'm not sure if it's the best solution, so I'm cc'ing some people who understand this better than me to take a look too.

1. The addition to the switch in ReplaceNodeResults() doesn't have a corresponding entry for X86ISD::SDIVREM8_SEXT_HREG (signed + sext). This isn't necessary currently because PerformSExtCombine() is missing the check for an MVT::i64 that exists in PerformZExtCombine() where these nodes are created. But I think that transform should be refactored into shared code, so they stay in sync. If we add that missing type check, we can crash for the SDIVREM case too, so we should handle SDIVREM in the same way as UDIVREM.

2. I know there are ~600 lines of precedence in crash.ll for not using FileCheck on crashing bugs, but I'd prefer that we check for codegen correctness with the new test case rather than the absence of a crash. "utils/update_llc_test_checks.py" should do well with the small test case; it's probably overkill for the big unreduced cases that already exist in crash.ll though, so you may want to split the regression test into its own or some other file.


http://reviews.llvm.org/D15331