[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
VyacheslavLevytskyy wrote:
Thank you for the PR! I'd like to better understand motivation and
justification of SPIR-V BE-related changes though. The goal would be to
understand whether AllSvmDevices is indeed a better choice (for whom?) than
Device as a default mem scope value in SPIR-V BE.
1) Questions to the description of the PR.
> "These were previously unconditionally lowered to Device scope, which is can
> be too conservative and possibly incorrect."
The claim is not justified by any docs/specs. Why Device scope is incorrect as
a default? In my opinion, it's AllSvmDevices that looks like a conservative
choice that may lead to performance degradation in general case when we change
the default without notifying customers. Or, we may say that potential
performance changes may depend on a vendor-specific behavior in this case.
> "Furthermore, the default / implicit scope is changed from Device (an OpenCL
> assumption) to AllSvmDevices (aka System), since the SPIR-V BE is not OpenCL
> specific / can ingest IR coming from other language front-ends."
What I know without additional references to other docs/specs is that Device is
default by OpenCL spec
(https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#atomic-functions).
It would help if you can provide references where AllSvmDevices is a
preferable choice, so that we are able to compare and figure out the best
default for the Computational flavor of SPIR-V. For sure, SPIR-V BE is not
OpenCL (=Device) specific, and it's also not specific to any particular vendor
or computational framework. I've seen usages of AllSvmDevices as default in the
code base (for example, in
https://github.com/llvm/llvm-project/blob/319e8cd201e6744199da377fba237dd276063e49/clang/lib/CodeGen/Targets/AMDGPU.cpp#L537),
but it seems not enough to flip the default over.
> "OpenCL defaulting to Device scope is now reflected in the front-end handling
> of atomic ops, which seems preferable."
Changes in clang part looks really good to me. However, when we add to it
changes in SPIR-V part of the code base, things look less optimistic, because
what this PR means by "the front-end handling of atomic ops" is the upstream
clang only, whereas actual choices of a front-end are more versatile, and users
coming to SPIR-V by other paths would get a sudden change of behavior in the
worst case (e.g., MLIR input for the GenAI domain).
===
2) If it's acceptable to split this PR into two separate PR's (clang and
SPIR-V), I'd gladly support changes in clang part, it makes sense for me. At
the moment, however, I have objections against SPIR-V Backend changes as they
are represented in the PR:
* This PR looks like a breaking change that would flip over the default value
of mem scope for all environments except for OpenCL and may have a potentially
negative impact on an unknown number of projects/customers. I'd guess that
OpenCL would not notice the difference, because path that goes via upstream
clang front-end redefines default mem scope as Device. All other toolchains
just get a breaking change in the form of the AllSvmDevices default.
clang-related changes do not help to smooth this, because SPIRV BE should
remain agnostic towards front-ends, frameworks, etc.
* A technical comment is that the proposed implementation in SPIR-V part is
less efficient that existing. It compares strings rather than integers and
fills in scope names on each call to the getMemScope() function, whereas
existing implementation does it just once per a machine function.
* A terminology (the choice of syncscope names) is debatable. The closest thing
in specs that I see is
https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_scope_id. I
don't see any references to "singlethread" in the specs. Name "workitem"
(spelling precisely as "work-item") is used at least in the official Khronos
documents (see for example
https://registry.khronos.org/SPIR-V/specs/1.0/SPIR-V-execution-and-memory-model.pdf).
"all_svm_devices" is not mentioned in the specs at all (there is only the
"CrossDevice" term).
===
For now, I'd rather see an eventual solution in the form of further
classification of the computational flavor of SPIR-V (not just Compute vs.
Vulkan but breaking Compute part further where this is required) -- comparing
to this sudden change of the default in favor of any incarnation of Compute
targets. As the first approach, all SPIR-V-related changes may require just a
short snippet of the kind "if TheTriple is XXX-specific then use CrossDevice
instead of Device" and minor rename of syncscope names ("subgroup", for
example, indeed makes more sense than "sub_group"). This would probably require
a description in the SPIRVUsage doc as well to avoid confusion among customers.
Anyway, I'd be glad to talk out a reasonable way forward to get a better
solution than we have now, if needed.
https://github.com/llvm/llvm-project/pull/106429
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?=
Message-ID:
In-Reply-To:
@@ -744,79 +744,139 @@ static void insertSpirvDecorations(MachineFunction &MF,
MachineIRBuilder MIB) {
MI->eraseFromParent();
}
-// Find basic blocks of the switch and replace registers in spv_switch() by its
-// MBB equivalent.
-static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
-MachineIRBuilder MIB) {
- DenseMap BB2MBB;
- SmallVector>>
- Switches;
+// LLVM allows the switches to use registers as cases, while SPIR-V required
+// those to be immediate values. This function replaces such operands with the
+// equivalent immediate constant.
+static void processSwitchesConstants(MachineFunction &MF,
+ SPIRVGlobalRegistry *GR,
+ MachineIRBuilder MIB) {
+ MachineRegisterInfo &MRI = MF.getRegInfo();
for (MachineBasicBlock &MBB : MF) {
-MachineRegisterInfo &MRI = MF.getRegInfo();
-BB2MBB[MBB.getBasicBlock()] = &MBB;
for (MachineInstr &MI : MBB) {
if (!isSpvIntrinsic(MI, Intrinsic::spv_switch))
continue;
- // Calls to spv_switch intrinsics representing IR switches.
- SmallVector NewOps;
- for (unsigned i = 2; i < MI.getNumOperands(); ++i) {
+
+ SmallVector NewOperands;
+ NewOperands.push_back(MI.getOperand(0)); // Opcode
+ NewOperands.push_back(MI.getOperand(1)); // Condition
+ NewOperands.push_back(MI.getOperand(2)); // Default
+ for (unsigned i = 3; i < MI.getNumOperands(); i += 2) {
Register Reg = MI.getOperand(i).getReg();
-if (i % 2 == 1) {
- MachineInstr *ConstInstr = getDefInstrMaybeConstant(Reg, &MRI);
- NewOps.push_back(ConstInstr);
-} else {
- MachineInstr *BuildMBB = MRI.getVRegDef(Reg);
- assert(BuildMBB &&
- BuildMBB->getOpcode() == TargetOpcode::G_BLOCK_ADDR &&
- BuildMBB->getOperand(1).isBlockAddress() &&
- BuildMBB->getOperand(1).getBlockAddress());
- NewOps.push_back(BuildMBB);
-}
+MachineInstr *ConstInstr = getDefInstrMaybeConstant(Reg, &MRI);
+NewOperands.push_back(
+MachineOperand::CreateCImm(ConstInstr->getOperand(1).getCImm()));
+
+NewOperands.push_back(MI.getOperand(i + 1));
}
- Switches.push_back(std::make_pair(&MI, NewOps));
+
+ assert(MI.getNumOperands() == NewOperands.size());
+ while (MI.getNumOperands() > 0)
+MI.removeOperand(0);
+ for (auto &MO : NewOperands)
+MI.addOperand(MO);
}
}
+}
+// Some instructions are used during CodeGen but should never be emitted.
+// Cleaning up those.
+static void cleanupHelperInstructions(MachineFunction &MF) {
SmallPtrSet ToEraseMI;
+ for (MachineBasicBlock &MBB : MF) {
+for (MachineInstr &MI : MBB) {
+ if (isSpvIntrinsic(MI, Intrinsic::spv_track_constant) ||
+ MI.getOpcode() == TargetOpcode::G_BRINDIRECT)
+ToEraseMI.insert(&MI);
+}
+ }
+
+ for (MachineInstr *MI : ToEraseMI)
+MI->eraseFromParent();
+}
+
+// Find all usages of G_BLOCK_ADDR in our intrinsics and replace those
+// operands/registers by the actual MBB it references.
+static void processBlockAddr(MachineFunction &MF, SPIRVGlobalRegistry *GR,
+ MachineIRBuilder MIB) {
+ // Gather the reverse-mapping BB -> MBB.
+ DenseMap BB2MBB;
+ for (MachineBasicBlock &MBB : MF)
+BB2MBB[MBB.getBasicBlock()] = &MBB;
+
+ // Gather instructions requiring patching. For now, only those can use
+ // G_BLOCK_ADDR.
+ SmallVector InstructionsToPatch;
+ for (MachineBasicBlock &MBB : MF) {
+for (MachineInstr &MI : MBB) {
+ if (isSpvIntrinsic(MI, Intrinsic::spv_switch) ||
+ isSpvIntrinsic(MI, Intrinsic::spv_loop_merge) ||
+ isSpvIntrinsic(MI, Intrinsic::spv_selection_merge))
+InstructionsToPatch.push_back(&MI);
+}
+ }
+
+ // For each instruction to fix, we replace all the G_BLOCK_ADDR operands by
+ // the actual MBB it references. Once those references updated, we can
cleanup
+ // remaining G_BLOCK_ADDR references.
SmallPtrSet ClearAddressTaken;
- for (auto &SwIt : Switches) {
-MachineInstr &MI = *SwIt.first;
-MachineBasicBlock *MBB = MI.getParent();
-SmallVector &Ins = SwIt.second;
+ SmallPtrSet ToEraseMI;
+ MachineRegisterInfo &MRI = MF.getRegInfo();
+ for (MachineInstr *MI : InstructionsToPatch) {
SmallVector NewOps;
-for (unsigned i = 0; i < Ins.size(); ++i) {
- if (Ins[i]->getOpcode() == TargetOpcode::G_BLOCK_ADDR) {
-BasicBlock *CaseBB =
-Ins[i]->getOperand(1).getBlockAddress()->getBasicBlock();
-auto It
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: https://github.com/VyacheslavLevytskyy edited https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
VyacheslavLevytskyy wrote: Thank you @AlexVlx for answering some of my questions. It was exactly my point in the comment that clang is not the only available FE and SPIRV BE should remain agnostic towards vendors, frameworks, etc., so it's nice to be on the same page wrt. vendor-specific choice of terminology (the use of "singlethread" is justified, but "all_svm_devices" is not) and/or the choice of default values (the System scope in the role of a stricter default is a reasonable justification). SPIRV BE needs to accommodate any flavor of SPIR-V Compute with equal hospitality, so, continuing your metaphor, that train still waits on the platform, it's just a matter of will and considerate manner of changes. Just a couple of comments: > Perhaps a better solution is to add an option that controls this behaviour / > the default, that the BE can consume and act accordingly? It doesn't seem necessary to get one more command line arg to cover this, it doesn't help more than setting an explicit scope in a (highly hypothetical) customer's code. > I believe the current string based design was put in place to give full > freedom to targets, so relying strictly on integer values for scopes is > legacy/less preferred. I don't agree. Existing implementation gives the same level of freedom to targets, using string values in the same way. However, we don't expect a target to change string names of scopes between getMemScope() calls, so there is no justification to use strings in every call instead of accounting for string names just once. I'd vote to keep the current approach as more performant. Talking about the best way to move forward, I don't see any contention points. I'd be glad to hear from you at the SPIR-V BE discussion group where it should be possible to talk this out faster/easier, or just here, both will do. Probably topics comprise just (1) the default value, (2) how to spell the word "all_svm_devices" properly to remain agnostic wrt. vendors (would it be "system"? "crossdevice"? anything else?), and (3) no reason behind getSyncScopeNames() asking string scope names on each call to getMemScope() instead of integers as now (i.e., if we indeed need to change the default, it looks a reasonable decision to integrate it with the existing code). https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: I didn't dive deeply into the subject, but I'd be eager to assist in a somewhat indirect manner. I've tried the PR locally to check this with SYCL and machine verifier (expensive checks: on). The former luckily didn't notice any changes, but the latter gives some insights. Probably I see one potential issue to address with respect to the validity of code between passes, namely after SPIRV pre legalizer. 37 of 63 test cases in the 'structurizer' folder are failing when running with 'llc -verify-machineinstrs', and it looks like the repeat offender is "Virtual register defs don't dominate all uses", like in ``` ... %45:iid(s32) = ASSIGN_TYPE %59:iid(s32), %56:type(s64) %95:iid(s32) = nsw G_ADD %96:iid, %97:iid %96:iid(s32) = GET_ID %45:iid(s32) %97:iid(s32) = GET_ID %26:iid(s32) %47:iid(s32) = nsw ASSIGN_TYPE %95:iid(s32), %56:type(s64) G_STORE %47:iid(s32), %5:pid(p0) :: (store (s32) into %ir.i) G_BR %bb.9 ... *** Bad machine code: Virtual register defs don't dominate all uses. *** - function:main - v. register: %97 ``` No existing tests with '-verify-machineinstrs' start to fail, so I guess there are no problems with code modifications wrt. the general/existing branching logic, but rather with the new use case introduced by the PR/added instructions. Another weird thing is that llvm specifically build with expensive checks on is now hangs (or maybe hits a very long timeout, I haven't yet a patience to check) while running our test suite, so I'm not quite sure if this PR introduce new issues for Machine Verifier in general, in the part of existing test cases where we haven't yet inserted '-verify-machineinstrs'. https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?= ,
Nathan =?utf-8?q?Gauër?=
Message-ID:
In-Reply-To:
@@ -744,79 +744,139 @@ static void insertSpirvDecorations(MachineFunction &MF,
MachineIRBuilder MIB) {
MI->eraseFromParent();
}
-// Find basic blocks of the switch and replace registers in spv_switch() by its
-// MBB equivalent.
-static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
-MachineIRBuilder MIB) {
- DenseMap BB2MBB;
- SmallVector>>
- Switches;
+// LLVM allows the switches to use registers as cases, while SPIR-V required
+// those to be immediate values. This function replaces such operands with the
+// equivalent immediate constant.
+static void processSwitchesConstants(MachineFunction &MF,
+ SPIRVGlobalRegistry *GR,
+ MachineIRBuilder MIB) {
+ MachineRegisterInfo &MRI = MF.getRegInfo();
for (MachineBasicBlock &MBB : MF) {
-MachineRegisterInfo &MRI = MF.getRegInfo();
-BB2MBB[MBB.getBasicBlock()] = &MBB;
for (MachineInstr &MI : MBB) {
if (!isSpvIntrinsic(MI, Intrinsic::spv_switch))
continue;
- // Calls to spv_switch intrinsics representing IR switches.
- SmallVector NewOps;
- for (unsigned i = 2; i < MI.getNumOperands(); ++i) {
+
+ SmallVector NewOperands;
+ NewOperands.push_back(MI.getOperand(0)); // Opcode
+ NewOperands.push_back(MI.getOperand(1)); // Condition
+ NewOperands.push_back(MI.getOperand(2)); // Default
+ for (unsigned i = 3; i < MI.getNumOperands(); i += 2) {
Register Reg = MI.getOperand(i).getReg();
-if (i % 2 == 1) {
- MachineInstr *ConstInstr = getDefInstrMaybeConstant(Reg, &MRI);
- NewOps.push_back(ConstInstr);
-} else {
- MachineInstr *BuildMBB = MRI.getVRegDef(Reg);
- assert(BuildMBB &&
- BuildMBB->getOpcode() == TargetOpcode::G_BLOCK_ADDR &&
- BuildMBB->getOperand(1).isBlockAddress() &&
- BuildMBB->getOperand(1).getBlockAddress());
- NewOps.push_back(BuildMBB);
-}
+MachineInstr *ConstInstr = getDefInstrMaybeConstant(Reg, &MRI);
+NewOperands.push_back(
+MachineOperand::CreateCImm(ConstInstr->getOperand(1).getCImm()));
+
+NewOperands.push_back(MI.getOperand(i + 1));
}
- Switches.push_back(std::make_pair(&MI, NewOps));
+
+ assert(MI.getNumOperands() == NewOperands.size());
+ while (MI.getNumOperands() > 0)
+MI.removeOperand(0);
+ for (auto &MO : NewOperands)
+MI.addOperand(MO);
}
}
+}
+// Some instructions are used during CodeGen but should never be emitted.
+// Cleaning up those.
+static void cleanupHelperInstructions(MachineFunction &MF) {
SmallPtrSet ToEraseMI;
+ for (MachineBasicBlock &MBB : MF) {
+for (MachineInstr &MI : MBB) {
+ if (isSpvIntrinsic(MI, Intrinsic::spv_track_constant) ||
+ MI.getOpcode() == TargetOpcode::G_BRINDIRECT)
+ToEraseMI.insert(&MI);
+}
+ }
+
+ for (MachineInstr *MI : ToEraseMI)
+MI->eraseFromParent();
+}
+
+// Find all usages of G_BLOCK_ADDR in our intrinsics and replace those
+// operands/registers by the actual MBB it references.
+static void processBlockAddr(MachineFunction &MF, SPIRVGlobalRegistry *GR,
+ MachineIRBuilder MIB) {
+ // Gather the reverse-mapping BB -> MBB.
+ DenseMap BB2MBB;
+ for (MachineBasicBlock &MBB : MF)
+BB2MBB[MBB.getBasicBlock()] = &MBB;
+
+ // Gather instructions requiring patching. For now, only those can use
+ // G_BLOCK_ADDR.
+ SmallVector InstructionsToPatch;
+ for (MachineBasicBlock &MBB : MF) {
+for (MachineInstr &MI : MBB) {
+ if (isSpvIntrinsic(MI, Intrinsic::spv_switch) ||
+ isSpvIntrinsic(MI, Intrinsic::spv_loop_merge) ||
+ isSpvIntrinsic(MI, Intrinsic::spv_selection_merge))
+InstructionsToPatch.push_back(&MI);
+}
+ }
+
+ // For each instruction to fix, we replace all the G_BLOCK_ADDR operands by
+ // the actual MBB it references. Once those references updated, we can
cleanup
+ // remaining G_BLOCK_ADDR references.
SmallPtrSet ClearAddressTaken;
- for (auto &SwIt : Switches) {
-MachineInstr &MI = *SwIt.first;
-MachineBasicBlock *MBB = MI.getParent();
-SmallVector &Ins = SwIt.second;
+ SmallPtrSet ToEraseMI;
+ MachineRegisterInfo &MRI = MF.getRegInfo();
+ for (MachineInstr *MI : InstructionsToPatch) {
SmallVector NewOps;
-for (unsigned i = 0; i < Ins.size(); ++i) {
- if (Ins[i]->getOpcode() == TargetOpcode::G_BLOCK_ADDR) {
-BasicBlock *CaseBB =
-Ins[i]->getOperand(1).getBlockAddress()->getBasicBlock();
-auto It
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: https://github.com/VyacheslavLevytskyy edited https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: https://github.com/VyacheslavLevytskyy edited https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: To be absolutely sure I've just cleanly rebuilt llvm with `-DLLVM_ENABLE_EXPENSIVE_CHECKS=ON` and I can confirm that our test suite is hanging/hitting a timeout now. It's slightly more worrying than 'llc -verify-machineinstrs' fails. https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: The culprit is `test/CodeGen/SPIRV/structurizer/cf.do.continue.ll`. https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL][DXIL][SPIRV] Create llvm dot intrinsic and use for HLSL (PR #102872)
@@ -380,6 +383,20 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg,
MIB.addImm(V);
return MIB.constrainAllUses(TII, TRI, RBI);
}
+
+ case TargetOpcode::G_FDOTPROD: {
+MachineBasicBlock &BB = *I.getParent();
+return BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpDot))
+.addDef(ResVReg)
+.addUse(GR.getSPIRVTypeID(ResType))
+.addUse(I.getOperand(1).getReg())
+.addUse(I.getOperand(2).getReg())
+.constrainAllUses(TII, TRI, RBI);
+ }
VyacheslavLevytskyy wrote:
I think trying to merge those two `SPIRV::OpDot`'s would not simplify or
improve anything, let's keep as is.
https://github.com/llvm/llvm-project/pull/102872
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL][SPIRV] Add any intrinsic lowering (PR #88325)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/88325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][SPIR-V] Add support for AMDGCN flavoured SPIRV (PR #89796)
@@ -114,15 +118,17 @@ Example: ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width. +``-target spirv64-amd-amdhsa`` can be used to compile for AMDGCN flavoured SPIR-V version 1.6 with 64-bit pointer width VyacheslavLevytskyy wrote: I'm not sure how the number of the version 1.6 will be deducted from just `spirv64-amd-amdhsa`. https://github.com/llvm/llvm-project/pull/89796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][SPIR-V] Add support for AMDGCN flavoured SPIRV (PR #89796)
@@ -0,0 +1,35 @@
+; RUN: llc -O0 -mtriple=spirv64-amd-amdhsa
--spirv-ext=+SPV_INTEL_function_pointers %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-amd-amdhsa %s -o -
-filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpCapability Int8
+; CHECK-DAG: OpCapability GenericPointer
+; CHECK-DAG: OpCapability FunctionPointersINTEL
+; CHECK-DAG: OpCapability Int64
+; CHECK: OpExtension "SPV_INTEL_function_pointers"
+; CHECK-DAG: %[[TyInt8:.*]] = OpTypeInt 8 0
+; CHECK-DAG: %[[TyVoid:.*]] = OpTypeVoid
+; CHECK-DAG: %[[TyInt64:.*]] = OpTypeInt 64 0
+; CHECK-DAG: %[[TyFunFp:.*]] = OpTypeFunction %[[TyVoid]] %[[TyInt64]]
+; CHECK-DAG: %[[ConstInt64:.*]] = OpConstant %[[TyInt64]] 42
+; CHECK-DAG: %[[TyPtrFunFp:.*]] = OpTypePointer Generic %[[TyFunFp]]
+; CHECK-DAG: %[[ConstFunFp:.*]] = OpConstantFunctionPointerINTEL
%[[TyPtrFunFp]] %[[DefFunFp:.*]]
+; CHECK: %[[FunPtr1:.*]] = OpBitcast %[[#]] %[[ConstFunFp]]
+; CHECK: %[[FunPtr2:.*]] = OpLoad %[[#]] %[[FunPtr1]]
+; CHECK: OpFunctionPointerCallINTEL %[[TyInt64]] %[[FunPtr2]] %[[ConstInt64]]
+; CHECK: OpReturn
+; CHECK: OpFunctionEnd
+; CHECK: %[[DefFunFp]] = OpFunction %[[TyVoid]] None %[[TyFunFp]]
+
+target triple = "spir64-unknown-unknown"
VyacheslavLevytskyy wrote:
Probably this line is not needed here?
https://github.com/llvm/llvm-project/pull/89796
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][SPIR-V] Add support for AMDGCN flavoured SPIRV (PR #89796)
@@ -0,0 +1,35 @@
+; RUN: llc -O0 -mtriple=spirv64-amd-amdhsa
--spirv-ext=+SPV_INTEL_function_pointers %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-amd-amdhsa %s -o -
-filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpCapability Int8
+; CHECK-DAG: OpCapability GenericPointer
+; CHECK-DAG: OpCapability FunctionPointersINTEL
+; CHECK-DAG: OpCapability Int64
+; CHECK: OpExtension "SPV_INTEL_function_pointers"
+; CHECK-DAG: %[[TyInt8:.*]] = OpTypeInt 8 0
+; CHECK-DAG: %[[TyVoid:.*]] = OpTypeVoid
+; CHECK-DAG: %[[TyFloat32:.*]] = OpTypeFloat 32
+; CHECK-DAG: %[[TyInt64:.*]] = OpTypeInt 64 0
+; CHECK-DAG: %[[TyPtrInt8:.*]] = OpTypePointer Generic %[[TyInt8]]
+; CHECK-DAG: %[[TyFunFp:.*]] = OpTypeFunction %[[TyFloat32]] %[[TyPtrInt8]]
+; CHECK-DAG: %[[TyFunBar:.*]] = OpTypeFunction %[[TyInt64]] %[[TyPtrInt8]]
%[[TyPtrInt8]]
+; CHECK-DAG: %[[TyPtrFunFp:.*]] = OpTypePointer Function %[[TyFunFp]]
+; CHECK-DAG: %[[TyPtrFunBar:.*]] = OpTypePointer Function %[[TyFunBar]]
+; CHECK-DAG: %[[TyFunTest:.*]] = OpTypeFunction %[[TyVoid]] %[[TyPtrInt8]]
%[[TyPtrInt8]] %[[TyPtrInt8]]
+; CHECK: %[[FunTest:.*]] = OpFunction %[[TyVoid]] None %[[TyFunTest]]
+; CHECK: %[[ArgFp:.*]] = OpFunctionParameter %[[TyPtrInt8]]
+; CHECK: %[[ArgData:.*]] = OpFunctionParameter %[[TyPtrInt8]]
+; CHECK: %[[ArgBar:.*]] = OpFunctionParameter %[[TyPtrInt8]]
+; CHECK: OpFunctionPointerCallINTEL %[[TyFloat32]] %[[ArgFp]] %[[ArgBar]]
+; CHECK: OpFunctionPointerCallINTEL %[[TyInt64]] %[[ArgBar]] %[[ArgFp]]
%[[ArgData]]
+; CHECK: OpReturn
+; CHECK: OpFunctionEnd
+
+target triple = "spir64-unknown-unknown"
VyacheslavLevytskyy wrote:
The same as above, I guess we don't need this line.
https://github.com/llvm/llvm-project/pull/89796
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][SPIR-V] Add support for AMDGCN flavoured SPIRV (PR #89796)
@@ -0,0 +1,35 @@
+; RUN: llc -O0 -mtriple=spirv64-amd-amdhsa
--spirv-ext=+SPV_INTEL_function_pointers %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-amd-amdhsa %s -o -
-filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpCapability Int8
+; CHECK-DAG: OpCapability GenericPointer
+; CHECK-DAG: OpCapability FunctionPointersINTEL
+; CHECK-DAG: OpCapability Int64
+; CHECK: OpExtension "SPV_INTEL_function_pointers"
+; CHECK-DAG: %[[TyInt8:.*]] = OpTypeInt 8 0
+; CHECK-DAG: %[[TyVoid:.*]] = OpTypeVoid
+; CHECK-DAG: %[[TyFloat32:.*]] = OpTypeFloat 32
+; CHECK-DAG: %[[TyInt64:.*]] = OpTypeInt 64 0
+; CHECK-DAG: %[[TyPtrInt8:.*]] = OpTypePointer Generic %[[TyInt8]]
+; CHECK-DAG: %[[TyFunFp:.*]] = OpTypeFunction %[[TyFloat32]] %[[TyPtrInt8]]
+; CHECK-DAG: %[[TyFunBar:.*]] = OpTypeFunction %[[TyInt64]] %[[TyPtrInt8]]
%[[TyPtrInt8]]
+; CHECK-DAG: %[[TyPtrFunFp:.*]] = OpTypePointer Function %[[TyFunFp]]
+; CHECK-DAG: %[[TyPtrFunBar:.*]] = OpTypePointer Function %[[TyFunBar]]
+; CHECK-DAG: %[[TyFunTest:.*]] = OpTypeFunction %[[TyVoid]] %[[TyPtrInt8]]
%[[TyPtrInt8]] %[[TyPtrInt8]]
+; CHECK: %[[FunTest:.*]] = OpFunction %[[TyVoid]] None %[[TyFunTest]]
+; CHECK: %[[ArgFp:.*]] = OpFunctionParameter %[[TyPtrInt8]]
+; CHECK: %[[ArgData:.*]] = OpFunctionParameter %[[TyPtrInt8]]
+; CHECK: %[[ArgBar:.*]] = OpFunctionParameter %[[TyPtrInt8]]
+; CHECK: OpFunctionPointerCallINTEL %[[TyFloat32]] %[[ArgFp]] %[[ArgBar]]
+; CHECK: OpFunctionPointerCallINTEL %[[TyInt64]] %[[ArgBar]] %[[ArgFp]]
%[[ArgData]]
+; CHECK: OpReturn
+; CHECK: OpFunctionEnd
+
+target triple = "spir64-unknown-unknown"
+
+define spir_kernel void @test(ptr addrspace(4) %fp, ptr addrspace(4) %data,
ptr addrspace(4) %bar) {
+entry:
+ %0 = call spir_func addrspace(4) float %fp(ptr addrspace(4) %bar)
+ %1 = call spir_func addrspace(4) i64 %bar(ptr addrspace(4) %fp, ptr
addrspace(4) %data)
+ ret void
+}
VyacheslavLevytskyy wrote:
I have a general suggestion, if I may, about those two function pointers tests.
Neither the extension itself is perfect, nor its current implementation in
SPIR-V Backend. About a half of Khronos Translator test cases for the function
pointers extension crash when running with our SPIR-V Backend implementation,
and this is a near to-do item in our development plans.
Giving all these, probably it's better to freeze in tests as little of function
pointers implementation logics as it's possible -- until the implementation is
completed in full and all Khronos Translator test cases are successful. In
terms of this PR this means that probably it's better to strip those 2 tests of
details whenever this is possible, unless you consider each existing CHECK in
those tests important.
https://github.com/llvm/llvm-project/pull/89796
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][SPIR-V] Add support for AMDGCN flavoured SPIRV (PR #89796)
https://github.com/VyacheslavLevytskyy approved this pull request. LGTM, thank you https://github.com/llvm/llvm-project/pull/89796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [libcxx] [llvm] [mlir] [SPIR-V] Introduce support of llvm.ptr.annotation to SPIR-V Backend and implement extensions which make use of spirv.Decorations (PR #93479)
Stefan =?utf-8?q?Gränitz?= ,Shengchen Kan ,David Green ,josel-amd <166385423+josel-...@users.noreply.github.com>,Matt Arsenault ,Kelvin Li ,Artem Kroviakov <71938912+akrovia...@users.noreply.github.com>,Benjamin Kramer ,Shengchen Kan ,Nico Weber ,Tom Eccles ,Sayan Saha ,Aaron Ballman ,Tyker ,Xu Zhang ,cor3ntin ,LLVM GN Syncbot ,Sander de Smalen ,"Levytskyy, Vyacheslav" Message-ID: In-Reply-To: https://github.com/VyacheslavLevytskyy converted_to_draft https://github.com/llvm/llvm-project/pull/93479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][HLSL] map lerp to Fmix (PR #88976)
@@ -58,4 +58,6 @@ let TargetPrefix = "spv" in {
Intrinsic<[ llvm_ptr_ty ], [llvm_i8_ty], [IntrWillReturn]>;
def int_spv_all : DefaultAttrsIntrinsic<[llvm_i1_ty], [llvm_any_ty]>;
def int_spv_any : DefaultAttrsIntrinsic<[llvm_i1_ty], [llvm_any_ty]>;
+ def int_spv_lerp : Intrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty,
LLVMMatchType<0>,LLVMMatchType<0>],
VyacheslavLevytskyy wrote:
Should we insert a description of the newly added intrinsic into
docs/SPIRVUsage.rst as a part of the same PR? What do you think
@michalpaszkowski ?
https://github.com/llvm/llvm-project/pull/88976
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][HLSL] map lerp to Fmix (PR #88976)
VyacheslavLevytskyy wrote: LGTM, just one question about documentation https://github.com/llvm/llvm-project/pull/88976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][HLSL] map lerp to Fmix (PR #88976)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/88976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][HLSL] Add mad intrinsic lowering for spirv (PR #89130)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/89130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen][SPIR-V][AMDGPU] Tweak AMDGCNSPIRV ABI to allow for the correct handling of aggregates passed to kernels / functions. (PR #102776)
VyacheslavLevytskyy wrote: > @VyacheslavLevytskyy @michalpaszkowski any objections / thoughts on this? Thank you for pinging me. I have no objections and no valuable advices wrt. this particular subject. https://github.com/llvm/llvm-project/pull/102776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: That's really great, thank you. > OpBranch/OpBranchConditional are not considered to be valid basic block > terminators In lib/Target/SPIRV/SPIRVInstrInfo.td lines 625-632 we actually set isTerminator=1 for both of those, but let's double check then. > ... leave the rest unfixed in this PR (which is already huge) Sure! https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: > OpLoopMerge are taking BB operands, but verifier expected register operand. Hopefully, the fix may be as simple as to change lib/Target/SPIRV/SPIRVInstrInfo.td line 620-621 `ID:$merge, ID:$continue` into `unknown:$merge, unknown:$continue` as in line 626 for OpBranch: `def OpBranch: Op<249, (outs), (ins unknown:$label), "OpBranch $label">;` https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -58,7 +58,35 @@ class SPIRVTargetCodeGenInfo : public
CommonSPIRTargetCodeGenInfo {
SPIRVTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
: CommonSPIRTargetCodeGenInfo(std::make_unique(CGT)) {}
void setCUDAKernelCallingConvention(const FunctionType *&FT) const override;
+ llvm::SyncScope::ID getLLVMSyncScopeID(const LangOptions &LangOpts,
+ SyncScope Scope,
+ llvm::AtomicOrdering Ordering,
+ llvm::LLVMContext &Ctx) const
override;
};
+
+inline StringRef mapClangSyncScopeToLLVM(SyncScope Scope) {
+ switch (Scope) {
+ case SyncScope::HIPSingleThread:
+ case SyncScope::SingleScope:
+return "singlethread";
+ case SyncScope::HIPWavefront:
+ case SyncScope::OpenCLSubGroup:
+ case SyncScope::WavefrontScope:
+return "subgroup";
+ case SyncScope::HIPWorkgroup:
+ case SyncScope::OpenCLWorkGroup:
+ case SyncScope::WorkgroupScope:
+return "workgroup";
+ case SyncScope::HIPAgent:
+ case SyncScope::OpenCLDevice:
+ case SyncScope::DeviceScope:
+return "device";
+ case SyncScope::SystemScope:
+ case SyncScope::HIPSystem:
+ case SyncScope::OpenCLAllSVMDevices:
+return "all_svm_devices";
VyacheslavLevytskyy wrote:
Should we return "" instead of "all_svm_devices" here as already existing name
denoting the System sync scope?
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
VyacheslavLevytskyy wrote: With the full respect to @AlexVlx work I created https://github.com/llvm/llvm-project/pull/108528 just as an utility to discuss and agree about memory scoped issues. https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -33,7 +33,8 @@
#include "llvm/Support/Debug.h"
namespace {
VyacheslavLevytskyy wrote:
@AlexVlx Let's remove all references to `struct SyncScopeIDs` from the whole
`SPIRVInstructionSelector.cpp` module. We will insert
`getOrInsertSyncScopeID()` in the `getMemScope()` portion of code.
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -251,6 +251,24 @@ SPIRV::MemorySemantics::MemorySemantics
getMemSemantics(AtomicOrdering Ord) {
llvm_unreachable(nullptr);
}
+SPIRV::Scope::Scope getMemScope(const LLVMContext &Ctx, SyncScope::ID ID) {
VyacheslavLevytskyy wrote:
@AlexVlx If you don't mind, let's do this function as the following:
```
SPIRV::Scope::Scope getMemScope(LLVMContext &Context, SyncScope::ID Id) {
// Named by
// https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_scope_id We
// don't need aliases for Invocation and CrossDevice, as we already have them
// covered by "singlethread" and "" strings respectively (see implementation
// of LLVMContext::LLVMContext()).
static llvm::SyncScope::ID SubGroupSSID =
Context.getOrInsertSyncScopeID("subgroup");
static llvm::SyncScope::ID WorkGroupSSID =
Context.getOrInsertSyncScopeID("workgroup");
static llvm::SyncScope::ID DeviceSSID =
Context.getOrInsertSyncScopeID("device");
if (Id == SyncScope::SingleThread)
return SPIRV::Scope::Invocation;
if (Id == SubGroupSSID)
return SPIRV::Scope::Subgroup;
if (Id == WorkGroupSSID)
return SPIRV::Scope::Workgroup;
if (Id == DeviceSSID)
return SPIRV::Scope::Device;
if (Id == SyncScope::System)
return SPIRV::Scope::CrossDevice;
// This function assumes that known memory sync scopes may appear to be the
// non-exhaustive list and allows to use the most conservative/safe choice as
// the default in case of an unknown SyncScope::ID value.
return SPIRV::Scope::CrossDevice;
}
```
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -251,6 +251,24 @@ SPIRV::MemorySemantics::MemorySemantics
getMemSemantics(AtomicOrdering Ord) {
llvm_unreachable(nullptr);
}
+SPIRV::Scope::Scope getMemScope(const LLVMContext &Ctx, SyncScope::ID ID) {
VyacheslavLevytskyy wrote:
I'd say that removal of SSIDs struct from
lib/Target/SPIRV/SPIRVInstructionSelector.cpp and this implementation of
`getMemScope()` may be enough. Here we may initialize and refer all scopes
concisely.
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -75,6 +75,8 @@ getMemSemanticsForStorageClass(SPIRV::StorageClass::StorageClass SC); SPIRV::MemorySemantics::MemorySemantics getMemSemantics(AtomicOrdering Ord); +SPIRV::Scope::Scope getMemScope(const LLVMContext &Ctx, SyncScope::ID ID); VyacheslavLevytskyy wrote: `SPIRV::Scope::Scope getMemScope(LLVMContext &Context, SyncScope::ID Id);` https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
VyacheslavLevytskyy wrote: @AlexVlx I don't see much objections against https://github.com/llvm/llvm-project/pull/108528 on a conceptual level, so what do you think about merging it into this PR in a way that I commented above, by changing getmemScope() and moving init into its static vars? https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
VyacheslavLevytskyy wrote: > > @AlexVlx I don't see much objections against #108528 on a conceptual level, > > so what do you think about merging it into this PR in a way that I > > commented above, by changing `getMemScope()` and moving > > `getOrInsertSyncScopeID()` into its static vars initialization? > > At a glance it seems fine, thank you for working through this, but since I've > been away for a couple of days let me page things back in first:) Sure, it's just that some parts of the discussion may be irrelevant if you'd choose the https://github.com/llvm/llvm-project/pull/108528 approach :) https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
@@ -1,12 +1,14 @@
; This test aims to check ability to support "Arithmetic with Overflow"
intrinsics
; in the special case when those intrinsics are being generated by the
CodeGenPrepare;
-; pass during translations with optimization (note -O3 in llc arguments).
+; pass during translations with optimization (note -disable-lsr, to inhibit
+; strength reduction pre-empting with a more preferable match for this pattern
+; in llc arguments).
-; RUN: llc -O3 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
-; RUN: %if spirv-tools %{ llc -O3 -mtriple=spirv32-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
+; RUN: llc -O3 -disable-lsr -mtriple=spirv32-unknown-unknown %s -o - |
FileCheck %s
VyacheslavLevytskyy wrote:
Happy to help with motivation. The original intent is to have a minimized
reproducer of an issue as a part of the test suite. The point of the test case
is to feed SPIR-V Backend with LLVM IR without saturation arithmetic intrinsics
and ensure that SPIR-V Backend is able to deal with those intrinsics appearing
along the way in translation. There is a separate test case in the test suite
for the same intrinsics explicitly represented in the input LLVM IR, and it's
another case, different from this one.
https://github.com/llvm/llvm-project/pull/110695
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
@@ -1,12 +1,14 @@
; This test aims to check ability to support "Arithmetic with Overflow"
intrinsics
; in the special case when those intrinsics are being generated by the
CodeGenPrepare;
-; pass during translations with optimization (note -O3 in llc arguments).
+; pass during translations with optimization (note -disable-lsr, to inhibit
+; strength reduction pre-empting with a more preferable match for this pattern
+; in llc arguments).
-; RUN: llc -O3 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
-; RUN: %if spirv-tools %{ llc -O3 -mtriple=spirv32-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
+; RUN: llc -O3 -disable-lsr -mtriple=spirv32-unknown-unknown %s -o - |
FileCheck %s
VyacheslavLevytskyy wrote:
In this case you lose coverage, because
> Correctly selecting / lowering them should be independent of whether they
> were implicitly inserted by some optimisation pass, or explicitly inserted by
> the user.
it's a nice idea, I agree with you, but it's somewhat detached. There are two
different conditions: (1) those intrinsics are present already and explicitly
accessible for processing by SPIRV Backend code, and (2) those intrinsics
appear during later passes and not accessible for processing. This test case is
related to the very concrete improvement.
https://github.com/llvm/llvm-project/pull/110695
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
@@ -1,12 +1,14 @@
; This test aims to check ability to support "Arithmetic with Overflow"
intrinsics
; in the special case when those intrinsics are being generated by the
CodeGenPrepare;
-; pass during translations with optimization (note -O3 in llc arguments).
+; pass during translations with optimization (note -disable-lsr, to inhibit
+; strength reduction pre-empting with a more preferable match for this pattern
+; in llc arguments).
-; RUN: llc -O3 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
-; RUN: %if spirv-tools %{ llc -O3 -mtriple=spirv32-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
+; RUN: llc -O3 -disable-lsr -mtriple=spirv32-unknown-unknown %s -o - |
FileCheck %s
VyacheslavLevytskyy wrote:
Thank you, I also agree that it doesn't seem appropriate to shift attention
from the PR to the test case.
> I'm not exactly sure what loss of coverage you have in mind here
> what the very concrete improvement is in this case
As I've said, "you have explicitly inserted llvm intrinsics in LLVM IR code"
and "you don't have it" means two different starting conditions for the
backend, and different options on how to implement their translation. It's an
implementation detail and is out of this PR's scope. Please feel free to ping
me aside of this discussion in the case if you interested in the subject and
would like to check implementation details and prior discussions.
https://github.com/llvm/llvm-project/pull/110695
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
https://github.com/VyacheslavLevytskyy approved this pull request. Thanks, sorry for the delay https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL][SPIRV][DXIL] Implement `dot4add_i8packed` intrinsic (PR #113623)
VyacheslavLevytskyy wrote: Thank you @inbelic , @s-perron , @dneto0 , it makes sense to me. https://github.com/llvm/llvm-project/pull/113623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL][SPIRV][DXIL] Implement `dot4add_i8packed` intrinsic (PR #113623)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/113623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] implement elementwise firstbithigh hlsl builtin (PR #111082)
@@ -0,0 +1,107 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-unknown %s -o - |
FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
+
+; CHECK: OpMemoryModel Logical GLSL450
+; CHECK: [[Z:%.*]] = OpConstant %[[#]] 0
VyacheslavLevytskyy wrote:
Let's do `CHECK-DAG` 2 times please here, there is no set order between those
OpConstant's.
https://github.com/llvm/llvm-project/pull/111082
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] implement elementwise firstbithigh hlsl builtin (PR #111082)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/111082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen][SPIR-V] Fix incorrect SYCL usage, implement missing interface (PR #109415)
VyacheslavLevytskyy wrote: @asudarsa Can you please have a look? https://github.com/llvm/llvm-project/pull/109415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -1,7 +1,7 @@ ; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV ; CHECK-SPIRV: %[[#Int:]] = OpTypeInt 32 0 -; CHECK-SPIRV-DAG: %[[#MemScope_Device:]] = OpConstant %[[#Int]] 1 +; CHECK-SPIRV-DAG: %[[#MemScope_AllSvmDevices:]] = OpConstant %[[#Int]] 0 VyacheslavLevytskyy wrote: I think it's better to be consistent in terminology of tests vs. source code, and to use here and in other tests CrossDevice instead of AllSvmDevices. https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -5,8 +5,8 @@
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
; CHECK: %[[#Int:]] = OpTypeInt 32 0
-; CHECK-DAG: %[[#Scope_Device:]] = OpConstant %[[#Int]] 1{{$}}
-; CHECK-DAG: %[[#MemSem_Relaxed:]] = OpConstant %[[#Int]] 0
+; CHECK-DAG: %[[#Scope_AllSvmDevices:]] = OpConstant %[[#Int]] 0{{$}}
VyacheslavLevytskyy wrote:
I'm not 100% sure, just as an idea -- maybe a choice of Const0 or ConstZero
would be a better way than slightly misleading use of Scope_AllSvmDevices in
the place of MemSem_Relaxed. Motivation for the idea is that `OpAtomicSMax
%[[#Int]] %[[#Pointer]] %[[#Scope_AllSvmDevices]] %[[#Scope_AllSvmDevices]]
%[[#Value]]` looks just wrong and I'm not sure that even the comment below will
help to avoid confusions.
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
https://github.com/VyacheslavLevytskyy edited https://github.com/llvm/llvm-project/pull/106429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -0,0 +1,163 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o -
| FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
+
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o -
| FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
+
+; CHECK: %[[#Int:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#Float:]] = OpTypeFloat 32
+; CHECK-DAG: %[[#Scope_AllSVMDevices:]] = OpConstant %[[#Int]] 0
+; CHECK-DAG: %[[#Value:]] = OpConstant %[[#Int]] 42
+; CHECK-DAG: %[[#FPValue:]] = OpConstant %[[#Float]] 42
+; CHECK-DAG: %[[#Scope_Invocation:]] = OpConstant %[[#Int]] 4
+; CHECK-DAG: %[[#MemSem_SeqCst:]] = OpConstant %[[#Int]] 16
+; CHECK-DAG: %[[#Scope_Subgroup:]] = OpConstant %[[#Int]] 3
+; CHECK-DAG: %[[#Scope_Workgroup:]] = OpConstant %[[#Int]] 2
+; CHECK-DAG: %[[#Scope_Device:]] = OpConstant %[[#Int]] 1
+; CHECK-DAG: %[[#PointerType:]] = OpTypePointer CrossWorkgroup %[[#Int]]
+; CHECK-DAG: %[[#FPPointerType:]] = OpTypePointer CrossWorkgroup %[[#Float]]
+; CHECK-DAG: %[[#Pointer:]] = OpVariable %[[#PointerType]] CrossWorkgroup
+; CHECK-DAG: %[[#FPPointer:]] = OpVariable %[[#FPPointerType]] CrossWorkgroup
+
+@ui = common dso_local addrspace(1) global i32 0, align 4
+@f = common dso_local local_unnamed_addr addrspace(1) global float
0.00e+00, align 4
+
+define dso_local spir_func void @test_singlethread_atomicrmw()
local_unnamed_addr {
+entry:
+ %0 = atomicrmw xchg i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicExchange %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %1 = atomicrmw xchg float addrspace(1)* @f, float 42.00e+00
syncscope("singlethread") seq_cst
+ ; CHECK: %[[#]] = OpAtomicExchange %[[#Float:]] %[[#FPPointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#FPValue:]]
+ %2 = atomicrmw add i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicIAdd %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %3 = atomicrmw sub i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicISub %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %4 = atomicrmw or i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicOr %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %5 = atomicrmw xor i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicXor %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %6 = atomicrmw and i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicAnd %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %7 = atomicrmw max i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicSMax %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %8 = atomicrmw min i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicSMin %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %9 = atomicrmw umax i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicUMax %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %10 = atomicrmw umin i32 addrspace(1)* @ui, i32 42 syncscope("singlethread")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicUMin %[[#Int]] %[[#Pointer:]]
%[[#Scope_Invocation:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+
+ ret void
+}
+
+define dso_local spir_func void @test_subgroup_atomicrmw() local_unnamed_addr {
+entry:
+ %0 = atomicrmw xchg i32 addrspace(1)* @ui, i32 42 syncscope("subgroup")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicExchange %[[#Int]] %[[#Pointer:]]
%[[#Scope_Subgroup:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %1 = atomicrmw xchg float addrspace(1)* @f, float 42.00e+00
syncscope("subgroup") seq_cst
+ ; CHECK: %[[#]] = OpAtomicExchange %[[#Float:]] %[[#FPPointer:]]
%[[#Scope_Subgroup:]] %[[#MemSem_SeqCst:]] %[[#FPValue:]]
+ %2 = atomicrmw add i32 addrspace(1)* @ui, i32 42 syncscope("subgroup")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicIAdd %[[#Int]] %[[#Pointer:]]
%[[#Scope_Subgroup:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %3 = atomicrmw sub i32 addrspace(1)* @ui, i32 42 syncscope("subgroup")
seq_cst
+ ; CHECK: %[[#]] = OpAtomicISub %[[#Int]] %[[#Pointer:]]
%[[#Scope_Subgroup:]] %[[#MemSem_SeqCst:]] %[[#Value:]]
+ %4 = atomicrmw or i32 addrspace(1)* @ui, i32 42 syncscope("subgroup") seq_cst
+ ; CHECK: %[[#]] = OpAtomicOr %[[#Int]] %[[#Pointer:]] %[[#Scope_Subgroup:]]
%[[#MemSem_SeqCst:]] %[[#Value:]]
+ %5 = atomicrmw xor
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -766,8 +766,19 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr
*Expr, Address Dest,
// LLVM atomic instructions always have synch scope. If clang atomic
// expression has no scope operand, use default LLVM synch scope.
if (!ScopeModel) {
+llvm::SyncScope::ID SS;
+if (CGF.getLangOpts().OpenCL)
+ // OpenCL approach is: "The functions that do not have memory_scope
+ // argument have the same semantics as the corresponding functions with
+ // the memory_scope argument set to memory_scope_device." See ref.:
+ //
https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#atomic-functions
+ SS = CGF.getTargetHooks().getLLVMSyncScopeID(CGF.getLangOpts(),
VyacheslavLevytskyy wrote:
May I ask you to add a test case to check specifically this line? That is to
check that coming from clang/OpenCL we get the `Device` memory scope when there
is no `synscope(...)`. We have llvm/test/CodeGen/SPIRV/atomicrmw.ll to check
SPIRV Backend default for no `syncscope`, but there is no test to cover the
OpenCL-specific default.
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -251,6 +251,24 @@ SPIRV::MemorySemantics::MemorySemantics
getMemSemantics(AtomicOrdering Ord) {
llvm_unreachable(nullptr);
}
+SPIRV::Scope::Scope getMemScope(const LLVMContext &Ctx, SyncScope::ID ID) {
VyacheslavLevytskyy wrote:
No problems, I'm quite happy with the way the suggestion is integrated, looks
as good, thank you.
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV][RFC] Rework / extend support for memory scopes (PR #106429)
@@ -58,7 +58,35 @@ class SPIRVTargetCodeGenInfo : public
CommonSPIRTargetCodeGenInfo {
SPIRVTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
: CommonSPIRTargetCodeGenInfo(std::make_unique(CGT)) {}
void setCUDAKernelCallingConvention(const FunctionType *&FT) const override;
+ llvm::SyncScope::ID getLLVMSyncScopeID(const LangOptions &LangOpts,
+ SyncScope Scope,
+ llvm::AtomicOrdering Ordering,
+ llvm::LLVMContext &Ctx) const
override;
};
+
+inline StringRef mapClangSyncScopeToLLVM(SyncScope Scope) {
+ switch (Scope) {
+ case SyncScope::HIPSingleThread:
+ case SyncScope::SingleScope:
+return "singlethread";
+ case SyncScope::HIPWavefront:
+ case SyncScope::OpenCLSubGroup:
+ case SyncScope::WavefrontScope:
+return "subgroup";
+ case SyncScope::HIPWorkgroup:
+ case SyncScope::OpenCLWorkGroup:
+ case SyncScope::WorkgroupScope:
+return "workgroup";
+ case SyncScope::HIPAgent:
+ case SyncScope::OpenCLDevice:
+ case SyncScope::DeviceScope:
+return "device";
+ case SyncScope::SystemScope:
+ case SyncScope::HIPSystem:
+ case SyncScope::OpenCLAllSVMDevices:
+return "";
+ }
VyacheslavLevytskyy wrote:
I would opt for adding explicitly defined behavior at the end of the function
(e.g., return "" as the fallback).
https://github.com/llvm/llvm-project/pull/106429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
@@ -1,56 +0,0 @@ -; This test aims to check ability to support "Arithmetic with Overflow" intrinsics VyacheslavLevytskyy wrote: @AlexVlx I'm strongly against deleting this test case. https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm][SPIR-V] Explicitly encode native integer widths for SPIR-V (PR #110695)
@@ -1,56 +0,0 @@ -; This test aims to check ability to support "Arithmetic with Overflow" intrinsics VyacheslavLevytskyy wrote: The main objection is that the code base should switch from one stable state to another, without losing current coverage, stability, etc. When any of us is adding a feature that alters translation behavior it looks fair to expect that the same contributor is responsible for updating all impacted existing components. Applying this principle to the PR, I'd rather expect that you update existing test case (if it's required by the change), but not remove it. https://github.com/llvm/llvm-project/pull/110695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen][SPIR-V] Fix incorrect SYCL usage, implement missing interface (PR #109415)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/109415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: Thank you @Keenuts! I'll check it today against my test suites and get back to you. I'd like to to check what's wrong with llvm/test/CodeGen/SPIRV/instructions/ret-type.ll very briefly. Meanwhile one more question, just out of curiosity. Between the pass of SPIRVPrepareFunctions.cpp with its new `sortBlocks(F)` and up to SPIRVEmitIntrinsicsPass, just before IRTranslator, we should see a lot of optimization going on when we run with -O1,-O2,-O3. I'm curious how `sortBlocks(F)` and other changes would behave when inserted before vs. after those optimizations applied? In other words, I wonder what is the best place to do this sort of transformation in SPIRV Backend passes and does it depend on with/without optimization choice. https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= , Nathan =?utf-8?q?Gauër?= Message-ID: In-Reply-To: VyacheslavLevytskyy wrote: All looks good! After this is merged I plan to have a look into: * this PR introduces new tests which fails with "extensive checks on", but they are not related to the PR's logic -- it's an existing issue with correct place to insert SPIRV type definitions required by assign type instructions, and * llvm/test/CodeGen/SPIRV/instructions/ret-type.ll fails due to reordering of return statements -- I see the reason, but it has no local and simple fix, so probably I should spend some time soon to improve this. https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Add SPIR-V structurizer (PR #107408)
Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= , Nathan =?utf-8?q?Gau=C3=ABr?= Message-ID: In-Reply-To: https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/107408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIR-V] Fixup storage class for global private (PR #116636)
Nathan =?utf-8?q?Gau=C3=ABr?= Message-ID: In-Reply-To: https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/116636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] Add Target Builtins using Distance ext as an example (PR #121598)
@@ -6757,6 +6757,8 @@ static Value *EmitTargetArchBuiltinExpr(CodeGenFunction *CGF, case llvm::Triple::riscv32: case llvm::Triple::riscv64: return CGF->EmitRISCVBuiltinExpr(BuiltinID, E, ReturnValue); + case llvm::Triple::spirv: +return CGF->EmitSPIRVBuiltinExpr(BuiltinID, E); VyacheslavLevytskyy wrote: @farzonl @michalpaszkowski I wonder should we extend this new approach to OpenCL as well? https://github.com/llvm/llvm-project/pull/121598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
VyacheslavLevytskyy wrote: > > I see also some problems in test cases > > @VyacheslavLevytskyy the SPIRV test case failures are not related to my > change. They are also failing on main. I will check now. For one the reason is clear, https://github.com/llvm/llvm-project/pull/123191 https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
VyacheslavLevytskyy wrote: > > > I see also some problems in test cases > > > > > > @VyacheslavLevytskyy the SPIRV test case failures are not related to my > > change. They are also failing on main. > > I will check now. For one the reason is clear, #123191 I can't reproduce two other fails on different environments, including actually Github Actions, so it maybe that after you rebase to `main` we will see all green now. https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
VyacheslavLevytskyy wrote: @farzonl Thank you for adding instcombine/pattern matching capabilities. LGTM overall. Maybe we should also document each transformation a bit in comments to the corresponding function, so that we clearly see what kind of input we expect to see to run the substitution? It could be anything in style of lib/Transforms/InstCombine/InstructionCombining.cpp comments (e.g., `(op (cast (op X, C2)), C1) --> (cast (op X, op (C1, C2)))`) or in style of more lengthy comments in lib/Target/SPIRV/SPIRVPreLegalizer.cpp (e.g., `propagateSPIRVType`). What do you think? https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
VyacheslavLevytskyy wrote: I see also some problems in test cases https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
VyacheslavLevytskyy wrote: @farzonl Was enabled GISelCSEAnalysisWrapperPass a reason of problems with inline_asm? https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
@@ -1,33 +1,44 @@
-; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
-; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o -
-filetype=obj | spirv-val %}
-
-; Make sure SPIRV operation function calls for distance are lowered correctly.
-
-; CHECK-DAG: %[[#op_ext_glsl:]] = OpExtInstImport "GLSL.std.450"
-; CHECK-DAG: %[[#float_16:]] = OpTypeFloat 16
-; CHECK-DAG: %[[#vec4_float_16:]] = OpTypeVector %[[#float_16]] 4
-; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32
-; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4
-
-define noundef half @distance_half4(<4 x half> noundef %a, <4 x half> noundef
%b) {
-entry:
- ; CHECK: %[[#]] = OpFunction %[[#float_16]] None %[[#]]
- ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_16]]
- ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_16]]
- ; CHECK: %[[#]] = OpExtInst %[[#float_16]] %[[#op_ext_glsl]] Distance
%[[#arg0]] %[[#arg1]]
- %spv.distance = call half @llvm.spv.distance.f16(<4 x half> %a, <4 x half>
%b)
- ret half %spv.distance
-}
-
-define noundef float @distance_float4(<4 x float> noundef %a, <4 x float>
noundef %b) {
-entry:
- ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]]
- ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]]
- ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]]
- ; CHECK: %[[#]] = OpExtInst %[[#float_32]] %[[#op_ext_glsl]] Distance
%[[#arg0]] %[[#arg1]]
- %spv.distance = call float @llvm.spv.distance.f32(<4 x float> %a, <4 x
float> %b)
- ret float %spv.distance
-}
-
-declare half @llvm.spv.distance.f16(<4 x half>, <4 x half>)
-declare float @llvm.spv.distance.f32(<4 x float>, <4 x float>)
+; RUN: llc -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
VyacheslavLevytskyy wrote:
Can we please add -verify-machineinstrs here to be sure that expensive_checks
run will be happy?
https://github.com/llvm/llvm-project/pull/122839
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
@@ -0,0 +1,140 @@ +; RUN: llc -mtriple=spirv-unknown-unknown -debug-pass=Structure < %s -o /dev/null 2>&1 | \ VyacheslavLevytskyy wrote: Just as a thought aloud, I wonder how stable this list can be at this stage of SPIR-V Backend development? I guess this test case may be subjected to more frequent changes than others. Also I think we'd anyway need to fix -O0 or -Osomething here. https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SPIRV] add pre legalization instruction combine (PR #122839)
VyacheslavLevytskyy wrote: > I rebased and the tests are passing locally. but they are still failing in ci. It's the strange part. The thing is when I created a new https://github.com/llvm/llvm-project/pull/123191 today, I see all tests passing in CI before I press Merged. That's why I thought that we can see the same here after rebase. This https://github.com/llvm/llvm-project/pull/123191 today was created basing on fresh main. So, it's a bit weird. https://github.com/llvm/llvm-project/pull/122839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIR-V] Use the SPIR-V backend by default (PR #129545)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/129545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIR-V] Use the SPIR-V backend by default (PR #129545)
VyacheslavLevytskyy wrote: > LGTM, thanks! In what regards the translator use in the HIP Toolchain, from > the AMD side we're looking at transitioning to the BE as soon as possible, > but it will probably be some time until we have fully validated it. I'd gladly hear about priorities (and maybe pain-points) w.r.t. the HIP toolchain when/if you are ready to share -- just to better understand where we are and what is to do. Potentially it may even appear that we have some items from the wish-list in common, who knows. https://github.com/llvm/llvm-project/pull/129545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SYCL][SPIR-V Backend][clang-sycl-linker] Add SPIR-V backend support inside clang-sycl-linker (PR #133967)
https://github.com/VyacheslavLevytskyy approved this pull request. https://github.com/llvm/llvm-project/pull/133967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
