This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Fix crash in RBS with a non-generic IMPLICIT_DEF.
ClosedPublic

Authored by aemerson on Mar 24 2021, 11:31 AM.

Details

Summary

This may occur when swifterror codegen in the translator generates these, but we shouldn't try to handle them since they should have regclasses anyway.

rdar://75784009

Diff Detail

Event Timeline

aemerson created this revision.Mar 24 2021, 11:31 AM
aemerson requested review of this revision.Mar 24 2021, 11:31 AM
aemerson updated this revision to Diff 333071.Mar 24 2021, 11:34 AM

Update x86 RBS test. For some reason it was using the non-generic IMPLICIT_DEF when it should be using the generic one.

arsenm added inline comments.Mar 24 2021, 11:36 AM
llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
723

How did this get past the isPreIselOpcode check (same with asm)?

aemerson added inline comments.Mar 24 2021, 11:44 AM
llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
723

Because the line above is short circuit AND, so it never calls isPreISelOpcode(). These opcodes are defined before the generic ones, so the isTargetSpecificOpcode(MI) call returns false.

paquette accepted this revision.Mar 24 2021, 2:05 PM

LGTM. Kind of weird that we create normal IMPLICIT_DEFs in the IRTranslator though? I guess it's code shared with SDAG.

This revision is now accepted and ready to land.Mar 24 2021, 2:05 PM

LGTM. Kind of weird that we create normal IMPLICIT_DEFs in the IRTranslator though? I guess it's code shared with SDAG.

Definitely shouldn't be doing that. No optimizations are going to recognize it

LGTM. Kind of weird that we create normal IMPLICIT_DEFs in the IRTranslator though? I guess it's code shared with SDAG.

Definitely shouldn't be doing that. No optimizations are going to recognize it

This falls under “already selected” code, and should have regclasses. The intention AFAIK is for GISel to allow targets to preselect code at any point in the pipeline.

LGTM. Kind of weird that we create normal IMPLICIT_DEFs in the IRTranslator though? I guess it's code shared with SDAG.

Definitely shouldn't be doing that. No optimizations are going to recognize it

This falls under “already selected” code, and should have regclasses. The intention AFAIK is for GISel to allow targets to preselect code at any point in the pipeline.

It should work, but it doesn't mean it's the best idea to use (especially for a "generic" instruction that has a direct "generic generic" instruction)