This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel][X86] G_ZEXT i1 to i32/i64 support.
ClosedPublic

Authored by igorb on May 8 2017, 6:07 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

igorb created this revision.May 8 2017, 6:07 AM
guyblank edited edge metadata.May 9 2017, 12:12 AM

General question, why do we need to support selection on i1? can't we widen it?

igorb added a comment.May 9 2017, 12:43 AM

General question, why do we need to support selection on i1? can't we widen it?

Widening done using G_ZEXT ( in some cases G_ANYEXT) so i need to support G_ZEXT selection in any case.

General question, why do we need to support selection on i1? can't we widen it?

Widening done using G_ZEXT ( in some cases G_ANYEXT) so i need to support G_ZEXT selection in any case.

ok

General question, why do we need to support selection on i1? can't we widen it?

Widening done using G_ZEXT ( in some cases G_ANYEXT) so i need to support G_ZEXT selection in any case.

i assume that the reason why use have custom code for lowering is that the tablegen patterns aren't fully supported. but once they will be supported, we'll need to have various patterns for illegal types as well? for G_ZEXT or any other instructions the legalize pass creates.

igorb added a comment.May 9 2017, 4:20 AM

i assume that the reason why use have custom code for lowering is that the tablegen patterns aren't fully supported. but once they will be supported, we'll need to have various patterns for illegal types as well? for G_ZEXT or any other instructions the legalize pass creates.

Yes.

guyblank added inline comments.May 9 2017, 5:20 AM
lib/Target/X86/X86LegalizerInfo.cpp
90 ↗(On Diff #98161)

is the SEXT legalization intentional?

igorb added inline comments.May 9 2017, 5:59 AM
lib/Target/X86/X86LegalizerInfo.cpp
90 ↗(On Diff #98161)

Yes, i think to mark G_SEXT i1 as legal operation and lower it to AND + NEG sequence in InstructionSelector.
G_SEXT Instruction selection not implemented in this patch.
relevant legalize testes for G_SEXT added.

guyblank added inline comments.May 9 2017, 6:41 AM
lib/Target/X86/X86LegalizerInfo.cpp
90 ↗(On Diff #98161)

ok, but you're missing the legalize test for s1->s64 sext

igorb updated this revision to Diff 98279.May 9 2017, 7:27 AM
igorb marked an inline comment as done.
  • add G_SEXT i1 to i64 legalize test Thanks!
guyblank accepted this revision.May 9 2017, 9:16 AM

LGTM

This revision is now accepted and ready to land.May 9 2017, 9:16 AM
This revision was automatically updated to reflect the committed changes.