[PATCH] D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND

[Nirav Dave via Phabricator via llvm-commits]

niravd added a comment.

Renato, apologies for the frustration. Had I not been in a rush to leave I would have realized my "please don't review yet" message hadn't been committed.

This started as a follow up fix for PR32284 (not 32884 as I typoed in the summary) which is a simple CombineTo ordering issue that caused use of deleted SDNodes causing assertions and could be solved with a little reordering. It was reopened after Dmitry found 100s large number of test failures of which he'd been able to package a few. Having a fix for that It was entirely unclear if I'd gotten them all and started this review as a rendezvous. I hadn't but the initial failure I saw looked unrelated so I planned on a follow up patch when I'd gotten a few more failures. But looking at the new failures this afternoon made it clear there was an ordering dependence on N0 and SetCCs which should be part of this patch.

At this point I'm waiting to hear if the current patch fixes the problem or if there's another testcase that has a similar failure.

I'm still uncertain what to do about tests. The nature of the assertion requires a certain amount of redundance in the input making using the auto-generated path relatively long as James commented. If we limited the tests to -O0 the output it should be fairly stable though and the differences should be obvious (and given these were all found with -O0 doesn't seem unreasonable). The make sure it doesn't assert approach seems reasonable as well.

Barring comments, my default plan is to mark the shared subset of instructions across the RUN calls that make sense.


https://reviews.llvm.org/D31625