Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amdgpu-isel appears to be the name for SelectionDAGISel for some targets? #59538

Closed
nickdesaulniers opened this issue Dec 15, 2022 · 4 comments
Closed

Comments

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Dec 15, 2022

I'm trying to write a test for some changes I'm making to SelectionDAGBuilder. The RUN: lines of my FileCheck tests on LLVM IR have llc -mtriple=aarch64-linux-gnu /tmp/x.ll -stop-after=finalize-isel -o - -global-isel=0 -fast-isel=0.

To isolate my test to testing isel, if I add: -start-before=finalize-isel, I no longer get any codegen from llc. It still prints the input IR and yaml for MIR, but the MIR output is gone.

If I replace -start-before=finalize-isel with -print-before-all, I see:

# *** IR Dump Before AArch64 Instruction Selection (amdgpu-isel) ***:

(huh, why is amdgpu-isel in there?)

On a hunch, testing -start-before=amdgpu-isel does what I'd have expected from -start-before=finalize-isel.

Expected:
llc -mtriple=aarch64-linux-gnu /tmp/x.ll -start-before=finalize-isel -stop-after=finalize-isel -o -
Actual:
llc -mtriple=aarch64-linux-gnu /tmp/x.ll -start-before=amdgpu-isel -stop-after=finalize-isel -o -

(Though, adding -print-before-all to Expected above shows what looks like amdgpu-isel, then aarch64-local-dynamic-tls-cleanup, then finalize-isel being run).

I see the same issue if I change the target triple to -mtriple=x86_64-linux-gnu as well, so not specific to aarch64 I think. So not tagging this as related to globalisel (since I don't want to test globalisel anyways).

cc @arsenm @aeubanks

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 15, 2022

@llvm/issue-subscribers-backend-amdgpu

@topperc topperc self-assigned this Dec 15, 2022
@nickdesaulniers
Copy link
Member Author

Thoughts from @topperc on discord:

my suspicious is its because static char ID; is in SelectionDAGISel instead of every one of it's target specific subclasses
so they all have the same identifying object
it might also be that amdgpu is the only instance of SelectionDAGISel that has an INITIALIZE_BEGIN

@nickdesaulniers
Copy link
Member Author

@nickdesaulniers nickdesaulniers self-assigned this Dec 19, 2022
nickdesaulniers added a commit that referenced this issue Dec 20, 2022
Quite a few passes are not default constructible. In order to properly
support -{start|stop}-{before|after}= for these passes, we would like to
continue to use INITIALIZE_PASS, but not necessarily provide a default
constructor.

Delete the default constructors of classes derived from
SelectionDAGISel.

Link: #59538

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D140349
nickdesaulniers added a commit that referenced this issue Dec 20, 2022
Follow a similar pattern as AMDGPUDAGToDAGISel's constructor so that we
can use INITIALIZE_PASS to register a pass. This allows for more fine
grain testability of SelectionDAGISel.

Link: #59538

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D140323
nickdesaulniers added a commit that referenced this issue Dec 21, 2022
Precommit sort and format of methods from D140331.

Link: https://reviews.llvm.org/D140331
Link: #59538
nickdesaulniers added a commit that referenced this issue Dec 21, 2022
…h64-isel

Follow a similar pattern as AMDGPUDAGToDAGISel's constructor so that we
can use INITIALIZE_PASS to register a pass. This allows for more fine
grain testability of SelectionDAGISel via:

   llc -stop-{before,after}=aarch64-isel

Link: #59538

See also: https://reviews.llvm.org/D140323

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D140331
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Dec 25, 2022
Quite a few passes are not default constructible. In order to properly
support -{start|stop}-{before|after}= for these passes, we would like to
continue to use INITIALIZE_PASS, but not necessarily provide a default
constructor.

Delete the default constructors of classes derived from
SelectionDAGISel.

Link: llvm/llvm-project#59538

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D140349
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Dec 25, 2022
Follow a similar pattern as AMDGPUDAGToDAGISel's constructor so that we
can use INITIALIZE_PASS to register a pass. This allows for more fine
grain testability of SelectionDAGISel.

Link: llvm/llvm-project#59538

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D140323
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Dec 25, 2022
Precommit sort and format of methods from D140331.

Link: https://reviews.llvm.org/D140331
Link: llvm/llvm-project#59538
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Dec 25, 2022
…h64-isel

Follow a similar pattern as AMDGPUDAGToDAGISel's constructor so that we
can use INITIALIZE_PASS to register a pass. This allows for more fine
grain testability of SelectionDAGISel via:

   llc -stop-{before,after}=aarch64-isel

Link: llvm/llvm-project#59538

See also: https://reviews.llvm.org/D140323

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D140331
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Dec 25, 2022
…maining targets

Follow up to the series:
1. https://reviews.llvm.org/D140161
2. https://reviews.llvm.org/D140349
3. https://reviews.llvm.org/D140331
4. https://reviews.llvm.org/D140323

Completes the work from the previous two for remaining targets.

This creates the following named passes that can be run via
`llc -{start|stop}-{before|after}`:
- arc-isel
- arm-isel
- avr-isel
- bpf-isel
- csky-isel
- hexagon-isel
- lanai-isel
- loongarch-isel
- m68k-isel
- msp430-isel
- mips-isel
- nvptx-isel
- ppc-codegen
- riscv-isel
- sparc-isel
- systemz-isel
- ve-isel
- wasm-isel
- xcore-isel

A nice way to write tests for SelectionDAGISel might be to use a RUN:
line like:
llc -mtriple=<triple> -start-before=<arch>-isel -stop-after=finalize-isel -o -

Fixes: llvm/llvm-project#59538

Reviewed By: asb, zixuan-wu

Differential Revision: https://reviews.llvm.org/D140364
nickdesaulniers added a commit that referenced this issue Feb 17, 2023
Given a CallBrInst, retain its first virtual register in SelectionDagBuilder's
FunctionLoweringInfo if there's corresponding landingpad. Walk the list
of COPY MachineInstr to find the original virtual and physical registers
defined by the INLINEASM_BR MachineInst.

Test cases from https://reviews.llvm.org/D139565.
Link: #59538

Part 3 from
https://discourse.llvm.org/t/rfc-syncing-asm-goto-with-outputs-with-gcc/65453/8

Follow up patches still need to wire up CallBrPrepare into the pass
pipelines.

Reviewed By: efriedma, void

Differential Revision: https://reviews.llvm.org/D140160
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Feb 17, 2023
Given a CallBrInst, retain its first virtual register in SelectionDagBuilder's
FunctionLoweringInfo if there's corresponding landingpad. Walk the list
of COPY MachineInstr to find the original virtual and physical registers
defined by the INLINEASM_BR MachineInst.

Test cases from https://reviews.llvm.org/D139565.
Link: llvm/llvm-project#59538

Part 3 from
https://discourse.llvm.org/t/rfc-syncing-asm-goto-with-outputs-with-gcc/65453/8

Follow up patches still need to wire up CallBrPrepare into the pass
pipelines.

Reviewed By: efriedma, void

Differential Revision: https://reviews.llvm.org/D140160
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 10, 2024
Quite a few passes are not default constructible. In order to properly
support -{start|stop}-{before|after}= for these passes, we would like to
continue to use INITIALIZE_PASS, but not necessarily provide a default
constructor.

Delete the default constructors of classes derived from
SelectionDAGISel.

Link: llvm/llvm-project#59538

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D140349
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 10, 2024
Follow a similar pattern as AMDGPUDAGToDAGISel's constructor so that we
can use INITIALIZE_PASS to register a pass. This allows for more fine
grain testability of SelectionDAGISel.

Link: llvm/llvm-project#59538

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D140323
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 10, 2024
Precommit sort and format of methods from D140331.

Link: https://reviews.llvm.org/D140331
Link: llvm/llvm-project#59538
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 10, 2024
…h64-isel

Follow a similar pattern as AMDGPUDAGToDAGISel's constructor so that we
can use INITIALIZE_PASS to register a pass. This allows for more fine
grain testability of SelectionDAGISel via:

   llc -stop-{before,after}=aarch64-isel

Link: llvm/llvm-project#59538

See also: https://reviews.llvm.org/D140323

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D140331
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 10, 2024
…maining targets

Follow up to the series:
1. https://reviews.llvm.org/D140161
2. https://reviews.llvm.org/D140349
3. https://reviews.llvm.org/D140331
4. https://reviews.llvm.org/D140323

Completes the work from the previous two for remaining targets.

This creates the following named passes that can be run via
`llc -{start|stop}-{before|after}`:
- arc-isel
- arm-isel
- avr-isel
- bpf-isel
- csky-isel
- hexagon-isel
- lanai-isel
- loongarch-isel
- m68k-isel
- msp430-isel
- mips-isel
- nvptx-isel
- ppc-codegen
- riscv-isel
- sparc-isel
- systemz-isel
- ve-isel
- wasm-isel
- xcore-isel

A nice way to write tests for SelectionDAGISel might be to use a RUN:
line like:
llc -mtriple=<triple> -start-before=<arch>-isel -stop-after=finalize-isel -o -

Fixes: llvm/llvm-project#59538

Reviewed By: asb, zixuan-wu

Differential Revision: https://reviews.llvm.org/D140364
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants