55c4ff91bd820d72014f63dcf7f3d5a0d3397986

6cb0d23f2ea6fb25106b0380797ccbc2141d71e1

e75afc9acf9b6de511c0c90b8e8a06364de46e3e

LGTM. Code should get better when we start trying to optimize bit ops

LGTM (although I think readfirstlane is the same cost as a regular copy)

LGTM, I've been meaning to do this. I do think some of these should be moved to combines and are missing hasOneUse checks though

dc08185ca797a3bcd7721a0d55db876a6cc4de10

a07759d04ab0f44c462b362df4df4d9a3a2f2b89

With the current subject, I don't think this would have received the appropriate attention. I think it should be reposted with a more generic subject for people more familiar with loop metadata

This could also just check the specific return opcodes (we could maybe even remove the isReturn from the shader pseudo-return)

In D87674#2290418, @madhur13490 wrote:

In D87674#2290396, @mareko wrote:

Yes, this commit is incorrect. It completely breaks code linking in Mesa OpenGL. s_waitcnt is required at the end of all global functions that return values.

Please revert. @nhaehnle

I don't understand why would it fail. This patch just moves s_waitcnt to the caller so they would be executed anyway. I think I am missing something. It would be helpful to root cause if we can isolate to a small test case.

c463fd136ec259ec269ee6741763ce595811da71

af0207f2bae8578c5283877a786e502ce6e33b14

ping

LGTM with test converted to generated checks

6daddc213fe56dccf1e88de61065c7fee09deccf

In D88028#2285683, @rampitec wrote:

Can this appear later in the codegen? It also does not cover global isel, so part in the operand folding probably needs to remain in addition to patterns.

All of this constant folding is really a DAG workaround

I assume this makes 1f4e7463b5e3ff654c84371527767830e51db10d redundant?

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947918, @hliao wrote:

They serve different purposes. Cross-reg-bank COPY is purely for value propagation as it must be materialized. But, coalescable COPY (on compatible register classes) needs to satisfy architectural constraints on different instructions and need to find a better register assignment to remove it if possible. The former needs eliminating redundant ones but the later needs skipping by most optimizations without comprehensive target-specific info. They need to be treated very differently.

c8757ff3aa7dd7a25a6343f6ef74a70c7be04325

3105d0f84bfa6b765bb88cbf090f557e588764ea

0576f436e577cede25810729aef236ec8c649446

About a year ago I was working on eliminating this code completely but I guess I won't be getting back to this any time soon

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947851, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947724, @arsenm wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947723, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947722, @arsenm wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947689, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947688, @arsenm wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947687, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947684, @arsenm wrote:

Can you revert this? I think that this transform itself is a workaround, and even if it were a good idea, I think it doesn't belong in another loop over the function in finalizeLowering

Could you elaborate on why that would be a workaround? Basically, after instruction selection, the COPY from SGPR to VGPR should be lowered to a native instruction.

Because this should be done after register allocation like is already done. Replacing a copy with something else should only interfere with generic optimizations

That would be too late for MachineCSE and other optimization to remove the redundant COPYs and reduce the register usage. Moving that after RA won't reduce register pressure.

So you are working around something MahcineCSE isn't doing on copies. You could just have MachineCSE do this for these copies

MachineCSE and other optimizations are designed not to handle that target-independent COPY or like, which is added with the intention that, potentially, the source and destination operands are coalesced and that COPY is removed finally. As SGPR and VGPR are different register banks and won't be coalesced anyway, native instruction should be used instead.

This does not make sense, COPY is what generic optimizations do understand. If it's useful to CSE cross bank copies, MachineCSE should handle them. As a representational choice, not-copy is worse than copy

A target-independent cross-bank COPY is definitely useful but the current COPY should not be used for that purpose considering how it's used in RA-related passes, especially only architectural constraints are changed between the source and destination operands where the propagation should be stopped.

In D87864#2281901, @rampitec wrote:

I think you need to drop denorm checks and move the check outside of the address space check.

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947724, @arsenm wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947723, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947722, @arsenm wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947689, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947688, @arsenm wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947687, @hliao wrote:

In rGc3492a1aa1b98c8d81b0969d52cea7681f0624c2#947684, @arsenm wrote:

Can you revert this? I think that this transform itself is a workaround, and even if it were a good idea, I think it doesn't belong in another loop over the function in finalizeLowering

Could you elaborate on why that would be a workaround? Basically, after instruction selection, the COPY from SGPR to VGPR should be lowered to a native instruction.

Because this should be done after register allocation like is already done. Replacing a copy with something else should only interfere with generic optimizations

That would be too late for MachineCSE and other optimization to remove the redundant COPYs and reduce the register usage. Moving that after RA won't reduce register pressure.

So you are working around something MahcineCSE isn't doing on copies. You could just have MachineCSE do this for these copies

MachineCSE and other optimizations are designed not to handle that target-independent COPY or like, which is added with the intention that, potentially, the source and destination operands are coalesced and that COPY is removed finally. As SGPR and VGPR are different register banks and won't be coalesced anyway, native instruction should be used instead.

This does not make sense, COPY is what generic optimizations do understand. If it's useful to CSE cross bank copies, MachineCSE should handle them. As a representational choice, not-copy is worse than copy

I ran into this before and apparently peephole optimizer should handle this: https://groups.google.com/g/llvm-dev/c/a4jKBqCJIDM/m/BRu3sWopBAAJ

ping

Add comment

751a6c5760b8de591cf241effbdad1b1cae67814

Why is a new entry block added in the first place?

I've reverted this in 27df1652709ba83d6b07f313297e7c796e36dce1