Page MenuHomePhabricator

[X86] Disable DomainReassignment pass when AVX512BW is disabled to avoid injecting VK32/VK64 references into the MachineIR
ClosedPublic

Authored by craig.topper on Jan 8 2019, 3:57 PM.

Details

Summary

This pass replaces GR8/GR16/GR32/GR64 with their equivalent sized mask register classes. But VK32/VK64 aren't legal without AVX512BW. Apparently this mostly appears to work if the register coalescer is able to remove the VK32/VK64 register class reference. Or if we don't ever spill it. But there's no guarantee of that.

Another Intel employee managed to trigger a crash due to this with ISPC. Unfortunately, I've lost the test case he sent me at the time. I'm trying to get him to reproduce it for me. I'd like to get this in before 8.0 branches since its a little scary.

The regressions here are unfortunate, but I think we can make some improvements to DAG combine, load folding, etc. to fix them. Just not sure if we can get that done for 8.0.

Fixes PR39741

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Jan 8 2019, 3:57 PM
craig.topper edited the summary of this revision. (Show Details)Jan 8 2019, 5:02 PM
spatel added a comment.Jan 9 2019, 7:09 AM

I still don't know much about AVX512, but code change is trivially safer. Upload patch with context for better future searchability?

lib/Target/X86/X86DomainReassignment.cpp
735 ↗(On Diff #180760)

of -> if

With context

RKSimon accepted this revision.Jan 9 2019, 1:59 PM

LGTM with the spelling mistake fix. Please raise bugs covering the regressions (missing folds etc.)

This revision is now accepted and ready to land.Jan 9 2019, 1:59 PM
This revision was automatically updated to reflect the committed changes.