This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel][X86] Support G_STORE/G_LOAD operation
ClosedPublic

Authored by igorb on Mar 15 2017, 2:59 AM.

Details

Summary
  1. Support pointer type as function argumnet and return value
  2. G_STORE/G_LOAD - set legal action for i8/i16/i32/i64/f32/f64/vec128
  3. RegisterBank - support typeless operations like G_STORE/G_LOAD, for scalar use GPR bank.
  4. Support instruction selection for G_LOAD/G_STORE

Diff Detail

Repository
rL LLVM

Event Timeline

igorb created this revision.Mar 15 2017, 2:59 AM
qcolombet edited edge metadata.Mar 15 2017, 7:42 AM

Hi Igor,

For G_LOAD, Fast and Greedy mode choose the same RegisterBank mapping (GprRegBank ) for the G_GLOAD + G_FADD , can't get rid of cross register bank copy GprRegBank->VecRegBank.

Yeah, we are aware of this problem. Greedy is not smart enough right now to do the right thing. I hope to fix that in the coming weeks.

I haven't looked at the patch, but is it something you worked around here?

Cheers,
-Quentin

igorb added a comment.Mar 15 2017, 8:07 AM

I haven't looked at the patch, but is it something you worked around here?

Cheers,
-Quentin

Hi Quentin,
No , It is straightforward implementation for G_LOAD/G_STORE operation.

rovka accepted this revision.Mar 23 2017, 3:35 AM

Hi,

Could you commit the unrelated changes separately (e.g. the space changes and the include reordering)?
Other than that, LGTM.

lib/Target/X86/X86InstructionSelector.cpp
321 ↗(On Diff #91846)

Why not else if, like the others?

test/CodeGen/X86/GlobalISel/x86_64-instructionselect.mir
659 ↗(On Diff #91846)

Can you also add testcases for s32 and s64 living in the vecr?

This revision is now accepted and ready to land.Mar 23 2017, 3:35 AM
igorb marked 2 inline comments as done.Mar 23 2017, 8:37 AM

Thanks for the review!

This revision was automatically updated to reflect the committed changes.