LGTM

Address comments

I'm fine with allowing vectors, but shouldn't we then also allow vectors for the G_*BFX? What was the reason for disallowing them there?

Thanks for the quick review @foad!

Add comment about range of operands

Rebase & ping

Only apply combine if the mask does not exceed in-memory size

Address review comments

Remove constexpr

Use available ShiftAmtTy

Rebase + test updates

ping @arsenm

In rGe892705d74c7366a1404a3b3471001edaa7659f8#1010526, @arsenm wrote:

In rGe892705d74c7366a1404a3b3471001edaa7659f8#1010520, @dsanders wrote:

In rGe892705d74c7366a1404a3b3471001edaa7659f8#1010512, @arsenm wrote:

In rGe892705d74c7366a1404a3b3471001edaa7659f8#1010405, @kschwarz wrote:

Is anyone already looking into supporting narrowing the result type below the memory size?

With this change, we cannot legalize %1:_(s128) = G_ZEXTLOAD %0:_(p0) :: (load (s64)) anymore, as s32 is the only legal result type for G_ZEXTLOAD on our target.

This is not a lowering, this is a narrowScalar of the result type. I'm trying to move towards narrowScalar never touching the memory type

I'm confused by this. You can't narrowScalar this below the memory type without splitting into multiple loads with smaller memory types since none of the load instructions are allowed to be truncating.

You can narrowScalar to a regular s64 load + zext. Whatever happens to that 64-bit load is a separate question

Is anyone already looking into supporting narrowing the result type below the memory size?

In D103815#2820179, @MaskRay wrote:

LGTM, but the subject and description need to be updated.

Remove the check for noload, adapt existing tests

https://reviews.llvm.org/D45264 specifically introduced the check for sec->noload, stating that this conforms to GNU ld 2.29

Thanks for your comments! I've applied the suggested formatting changes.

Test formatting

Is there a plan to eventually get rid of the EVTs in call lowering and directly work with the LLVM IR Type?
Our downstream target needs special handling for pointer arguments, but unfortunately ComputeValueVTs loses the information about pointerness.
We solved this by directly operating on the LLVM IR type, although we do not handle vectors yet, which makes this a bit simpler

Adapt to new API of getConstantVRegValWithLookThrough, rebase

In D89480#2332584, @arsenm wrote:

Duplicate D76500

Yes, I'll commit it tomorrow, thanks for the review @akyrtzi!

Address review comments

In D89100#2322546, @arsenm wrote:

The code asserting in GISelKnownBits is probably broken. An out of bounds shift is not illegal IR

In D85199#2210127, @arsenm wrote:

In D85199#2193484, @arsenm wrote:

In D85199#2193321, @kschwarz wrote:

The problem we have is that some LLVM-IR passes introduce calls to llvm.memcpy.p0i8.p0i8.i64, even though our native pointer size is 32-bits. Since it turned out that fixing this in LLVM is quite invasive, we currently work around this problem by truncating the size argument to 32-bits before calling createMemLibcall.

You would have to deal with this whether passes introduce this IR or not, unless we make it a verifier error to deviate from the pointer size (which would be inconsistent with every other pointer sized integer operand, although I think this is pointless)

I'm thinking the IRTranslator should be responsible for this clamp, and the machine verifier should enforce the size of the length type. I'm not sure why we don't do this for G_INTTOPTR/G_PTRTOINT; I feel like the fact these aren't required to match the pointer size is a holdover from when the DataLayout was optional

The promotion gets inserted in the libcall lowering [...]

Either you can create a bug in Bugzilla (https://bugs.llvm.org/), asking for this revision to be merged to the release branch and mark it as a release blocker of the 11.0.0 release, or you can contact the release manager directly (hans@chromium.org).

LGTM, might be worth to include this also for the 11.x release.

Rebase

A similar issue (for tied input operands) is handled in https://reviews.llvm.org/D83384
The function introduced there should be extended to handle the vector case.

we noticed that this extension is also missing for the non-tied register input operands. I added the opposite logic, i.e. truncating of output operands, but forgot about extending the inputs.
Would you mind adding that handling in this patch too, as it is basically the same?
If you feel like it should be in a separate commit, I can also provide a patch for it. Thanks!

I brought this to the mailing list (http://lists.llvm.org/pipermail/llvm-dev/2020-March/140032.html) but didn't get an answer there.

Brining this up again

Yes, abandoning this in favor of https://reviews.llvm.org/D78316

Hi @leonardchan, I'll take a look at the failure, thanks for the reproducer!

Re-landed in 5425cdc3adf9998aeaf587d93417bd2f4f1373c9

The new check also triggered for memory clobbers, which we already handle.
I've moved the check further down, where we already check that it is an input operand.

Re-opening for investigation

Add test extracted from pr68328.c

I did not handle direct memory operands that need to be "indirectified" in this patch.
https://reviews.llvm.org/D79955 adds an early return for now so we don't run into the assertion while working on supporting that feature.

I'll have a look into it now. Thanks for letting me know!