[PATCH] D96906: [AMDGPU] gfx90a support

rampitec marked an inline comment as done. rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:100 bool tryFoldOMod(MachineInstr ); + bool tryFoldRegSeqence(MachineInstr ); + bool tryFoldLCSSAPhi(MachineInstr ); foad wrote:

[PATCH] D96906: [AMDGPU] gfx90a support

foad added inline comments. Herald added a subscriber: mstorsjo. Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:100 bool tryFoldOMod(MachineInstr ); + bool tryFoldRegSeqence(MachineInstr ); + bool tryFoldLCSSAPhi(MachineInstr ); Spelling "sequence".

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } rampitec wrote:

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } arsenm wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } rampitec wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } rampitec wrote:

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } arsenm wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } rampitec wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } arsenm wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } rampitec wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

foad added a comment. In D96906#2573265 , @echristo wrote: > In D96906#2572842 , @msearles wrote: > >> In D96906#2572749 , @kzhuravl wrote: >> The point is that nobody

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: clang/include/clang/Driver/Options.td:3097-3101 +def mtgsplit : Flag<["-"], "mtgsplit">, Group, + HelpText<"Enable threadgroup split execution mode (AMDGPU only)">; +def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group, +

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: llvm/include/llvm/BinaryFormat/ELF.h:737 EF_AMDGPU_MACH_AMDGCN_FIRST = EF_AMDGPU_MACH_AMDGCN_GFX600, - EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX805, + EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX90A,

[PATCH] D96906: [AMDGPU] gfx90a support

echristo added a comment. In D96906#2572842 , @msearles wrote: > In D96906#2572749 , @kzhuravl wrote: > >>> The point is that nobody upstream even got a chance to chime in. >> >> We are and will be taking care of

[PATCH] D96906: [AMDGPU] gfx90a support

tra added a comment. In D96906#2572842 , @msearles wrote: > In D96906#2572749 , @kzhuravl wrote: > >>> The point is that nobody upstream even got a chance to chime in. >> >> We are and will be taking care of any

[PATCH] D96906: [AMDGPU] gfx90a support

msearles added a comment. In D96906#2572749 , @kzhuravl wrote: >> The point is that nobody upstream even got a chance to chime in. > > We are and will be taking care of any feedback provided in this review > post-commit. To be fair to @rampitec , it was

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added a comment. > The point is that nobody upstream even got a chance to chime in. We are and will be taking care of any feedback provided in this review post-commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96906/new/

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } arsenm wrote: >

[PATCH] D96906: [AMDGPU] gfx90a support

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199 + MCRegister RepReg; + for (MCRegister R : *MRI->getRegClass(Reg)) { +if (!MRI->isReserved(R)) { + RepReg = R; + break; +} + } This is a problem

[PATCH] D96906: [AMDGPU] gfx90a support

kzhuravl added inline comments. Comment at: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.workitem.id.ll:23 + +; CO-V3: .amdhsa_system_vgpr_workitem_id 0 +; PACKED-TID: .amdhsa_system_vgpr_workitem_id 0 foad wrote: > CO-V3 isn't tested by any RUN line. I think FileCheck

[PATCH] D96906: [AMDGPU] gfx90a support

foad added inline comments. Comment at: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.workitem.id.ll:23 + +; CO-V3: .amdhsa_system_vgpr_workitem_id 0 +; PACKED-TID: .amdhsa_system_vgpr_workitem_id 0 CO-V3 isn't tested by any RUN line. I think FileCheck might complain

[PATCH] D96906: [AMDGPU] gfx90a support

tra added a comment. In D96906#2570095 , @rampitec wrote: > It's a year of work necessarily downstream. Every line there was reviewed and > tested in the downstream. I understand no one can reasonably review something > that big, although I cannot break

[PATCH] D96906: [AMDGPU] gfx90a support

rampitec added a comment. In D96906#2570086 , @tra wrote: > This is a pretty huge patch, with no details in the commit log. > > One hour between sending the patch out and landing it is not sufficient for > anyone to meaningfully > review the patch and

[PATCH] D96906: [AMDGPU] gfx90a support

tra added a comment. This is a pretty huge patch, with no details in the commit log. One hour between sending the patch out and landing it is not sufficient for anyone to meaningfully review the patch and there are no mentions of the review done anywhere else. While the code only changes

[PATCH] D96906: [AMDGPU] gfx90a support

This revision was automatically updated to reflect the committed changes. Closed by commit rGa8d9d50762c4: [AMDGPU] gfx90a support (authored by rampitec). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: