In D62198#1510400, @nhaehnle wrote:

I'm personally not confident enough in this change to enable it unconditionally immediately. I think this should be guarded with an option.

Guarded the change by an option to allow users of the structurizer to turn this functionality on in their own time.

In D60600#1485496, @reames wrote:

In D60600#1475212, @sheredom wrote:

In D60600#1470778, @reames wrote:

I think you might be overcomplicating this. I'd suggest the following:

1. scan all index types for a gep
2. run current code, but if any operand was gep, skip the recursive call on the operand
   
   It's slightly less powerful, but correct, and easy. And frankly, optimize geps of struct types is probably rare enough we don't care.
   
   p.s. Agreed on the location of test file, please add to existing tests.

I don't really understand what your suggestion is here sorry - let me try and rephrase the suggestion and see if we agree?

I submitted a version along what I proposed as https://reviews.llvm.org/rL359633. Please confirm that fixes your crash. If you want to work on a better version of the fix, I'm happy to review, but I wanted to make sure the crash was resolved.

Added the extra test case that @spatel found that my change also fixes.

@spatel yup its the same bug, and my fix fixes it to. I'll add it to my test changes.

@reames ping?

Fix some review comments by @spatel

In D60600#1470778, @reames wrote:

I think you might be overcomplicating this. I'd suggest the following:

1. scan all index types for a gep
2. run current code, but if any operand was gep, skip the recursive call on the operand
   
   It's slightly less powerful, but correct, and easy. And frankly, optimize geps of struct types is probably rare enough we don't care.
   
   p.s. Agreed on the location of test file, please add to existing tests.

Found a fun little bug whereby the phys vgprs were being coalesced onto previous instructions, and then shouldClusterMemOps was assuming only virt regs. Added a workaround for that.

Update for two reasons:

Fix review comments.

Added the max3 cases too.

Changed it to a whitelist of types rather than a blacklist (much better idea).

Fixed review comments by Matt and added the extra test cases that were a great idea!

Reduce the number of DPP calls in the test for cleanliness, and reintroduce the convergent on WWM. The CFG test contains the bug that was exposed by the lack of convergent on WWM, LLVM will sink the WWM statement out of the branch which totally messes up all calculations.

Remove the explicit pass name.

Addressed all review comments.

Add a test case that triggers the target transform info code path.

LGTM.

LGTM.

In D57737#1392667, @tpr wrote:

But I still don't understand it:

1. Why do you want an exclusive scan? Surely what you're trying to do is just "sum" up all lanes into lane 63, which is an inclusive scan.

Fix comment that should now say exclusive scan instead of inclusive scan.

Make the final readlane be in the WWM section too as per the review comments.

Updated to bring in an additional fix to remove read_register exec and replace it with a ballot.

Fixed review comments.

Fix review comments.

Review comments.

Also tag s.getpc as scalar, and add tests in the Analysis folder like suggested.

@arsenm agreed - not quite sure how to do tests for this? I started to look at how we test alias analysis, and they have a cl::opt to force print the debug info for that pass, and that is how our amdgpu-alias-analysis.ll tests the aliasing rules. How exactly to test that divergence analysis did something here?

@cwabbott I planned to do a followup once this DPP change had landed to add the missing dpp/codegen patterns to the atomic optimizer - so watch this space!

Fix review comment.

Fix review comments.

@nhaehnle I've removed canReadVGPR as it only had the single callsite - but given @arsenm's comment I did not change getOpRegClass. I also don't feel super comfortable doing the change you suggested to addUsersToMoveToVALUWorklist as I'm worried that there will be non-LLVM-tested paths that I could trip up on with ease. I'd rather ship the commit as is if y'all are ok with it.

Removed canReadVGPR as it had only a single callsite.

Made the l1 flush change happen for MESA3D too (like Nicolai asked for).

Fixed review comments: