Test fixes

Missed one

This will help atomic store handling for global isel. Right now there's a complication because ISD::ATOMIC_STORE swaps the operand order from a normal store

LGTM

I think the standard for what is committed for the triple is quite low. We already have out of tree targets included in the upstream triple, so it doesn't really matter what happens with the follow up patches. This should be fine to commit without waiting for the rest of the backend review

LGTM

Fix for SI

I'm somewhat confused by what the responsibilities of handleAssignments are vs. target code. I already have multiple register breakdowns without this patch in AMDGPU

r369203

r369203

ping

Use common tablegen test target

Manually fix hexagon

In D64931#1633669, @akhuang wrote:

Address space have backend defined semantics, and aren’t really reserved for front end use. I think the fact that non-0 address spaces on X86 codegen the same as address space 0 and could be used for something by a front end is an accident of how SelectionDAG is implemented. If X86 wants to reserve address space ranges for frontend use, that would need to be decided and documented. You don’t necessarily get the current behavior for free in GlobalISel since pointer types are distinct, so this would specifically need to be implemented.

By this do you mean that this would be an instance of address spaces being used by the frontend? Or just that adding meaning to address spaces shouldn't be breaking other frontends?

r369038

In D65542#1631857, @jsji wrote:

@arsenm Is this MIR test OK?

ping

LGTM

In D66197#1629896, @jdoerfert wrote:

In D66197#1629864, @arsenm wrote:

In D66197#1629861, @jdoerfert wrote:

In D66197#1629809, @b-sumner wrote:

In D66197#1629784, @jdoerfert wrote:

Do we really need these to be "amdgpu" specific?

Are you envisioning these would be used for OpenCL implementations? OpenCL doesn't exactly have these. It instead has to_<addrspacename> functions which return NULL if the generic pointer isn't actually pointing at a object in <addrrspacename>.

We (will) have various languages/targets that have corresponding address spaces and we already reserve some numbers for specific address spaces (afaik), why not make this a generic functionality.

The IR doesn't reserve any numbers for specific usage (except 0 has some special properties, which do not include being a flat/generic pointer as defined in OpenCL). It might make more sense to add a clang builtin for this, which could then be implemented with the target specific intrinsic. I don't want to add a generic target intrinsic while guessing at how this might work on other targets. Something truly generic, like llvm.is.address.space(ptr, address_space_id) I don't think really works generally enough to add. There isn't necessarily a 1:1 mapping between the language address space and IR address space. There could possibly be multiple IR address spaces to handle, and not all targets might be able to do this test at all

I see, still, we have llvm.nvvm.isspacep.const and friends already. Now we get llvm.amdgcn.is.private, which seems to be the same thing for amdgpu. As long as people only use this in the backend, great, but if we want middle-end passes that deal with address spaces and optimize accordingly, e.g., introduce data movement, we should have generic intrinsics or helper functions. I would prefer the former and I was curious if there is a problem with that. If the folding (I described earlier) is triple/target specific, sure, but if we do not have multiple llvm.XYZ.is.private we would simplify things.

In D66197#1629861, @jdoerfert wrote:

In D66197#1629809, @b-sumner wrote:

In D66197#1629784, @jdoerfert wrote:

Do we really need these to be "amdgpu" specific?

Are you envisioning these would be used for OpenCL implementations? OpenCL doesn't exactly have these. It instead has to_<addrspacename> functions which return NULL if the generic pointer isn't actually pointing at a object in <addrrspacename>.

We (will) have various languages/targets that have corresponding address spaces and we already reserve some numbers for specific address spaces (afaik), why not make this a generic functionality.

r368895

In D66197#1629784, @jdoerfert wrote:

Do we really need these to be "amdgpu" specific?

rename

Rename

Also move operand collection to TTI

In D66197#1629348, @b-sumner wrote:

Looks fine to me. Thanks!

LGTM, although I copied this from the DAG lowering, which does seem to use the result number of elements?

LGTM, although I rather dislike that we allow these. Maybe the IRTranslator should be trying to turn these into the equivalent G_BUILD_VECTOR?

ping

ping

LGTM

r368713

ping

ping

r368709

r368705

In D66109#1627112, @aditya_nandakumar wrote:

In D66109#1626195, @arsenm wrote:

In D66109#1626112, @aditya_nandakumar wrote:

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Not sure where that would go

Perhaps in the bottom of https://www.llvm.org/docs/GlobalISel.html ? Alternatively we can document in GenericOpcodes.td right along the instructions which have links.

I added a note to GenericOpcodes.td

r368704

LGTM

Fix size

LGTM

Use buildBiuildVector