Page MenuHomePhabricator

[GlobalISel] add additional lowering support for G_INSERT
ClosedPublic

Authored by gargaroff on Mar 11 2020, 8:16 AM.

Details

Summary

Add lowering support for inserting pointers or scalars into scalars, vectors or pointers

Diff Detail

Event Timeline

gargaroff created this revision.Mar 11 2020, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2020, 8:16 AM
gargaroff updated this revision to Diff 249631.Mar 11 2020, 8:21 AM

Use buildCast for better readability

arsenm added inline comments.Mar 11 2020, 9:23 AM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
4855–4858

This should probably fail for a non integral address space

4855–4860

Indentation mess. Move the scalar type to a variable, and add braces?

gargaroff updated this revision to Diff 249890.Mar 12 2020, 4:05 AM

Clean up formatting. Fail on non-integral address spaces

gargaroff updated this revision to Diff 249891.Mar 12 2020, 4:08 AM

Fix clang-tidy errors in unit test

gargaroff marked 2 inline comments as done.Mar 12 2020, 4:10 AM

I added a check to fail for non-integral address space pointers. I couldn't add any tests since the datalayout used in the unit tests doesn't have any non-integral address space pointers (or at least I didn't see any). Hope that's not a problem.

Harbormaster completed remote builds in B48965: Diff 249891.
arsenm accepted this revision.Mar 16 2020, 7:17 AM
arsenm added inline comments.
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-insert.mir
1298

Should this have been triggering a verifier error? Is there a missing check for source type == dest type?

This revision is now accepted and ready to land.Mar 16 2020, 7:17 AM
gargaroff closed this revision.Mar 16 2020, 8:29 AM
gargaroff marked an inline comment as done.
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-insert.mir
1298

I guess so. I'd say that casting only makes sense when the types change. Might be worth to look into in a follow up.

dsanders added inline comments.Mar 16 2020, 10:17 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-insert.mir
1298

I agree. G_BITCAST shouldn't be valid for the same type on operand and result