================
@@ -1444,15 +1444,16 @@ void clang::emitBackendOutput(CompilerInstance &CI,
CodeGenOptions &CGOpts,
// Verify clang's TargetInfo DataLayout against the LLVM TargetMachine's
// DataLayout.
- if (AsmHelper.TM) {
- std::string DLDesc = M->getDataLayout().getStringRepresentation();
- if (DLDesc != TDesc) {
+ if (AsmHelper.TM)
+ if (!AsmHelper.TM->isCompatibleDataLayout(M->getDataLayout()) ||
+ !AsmHelper.TM->isCompatibleDataLayout(DataLayout(TDesc))) {
+ std::string DLDesc = M->getDataLayout().getStringRepresentation();
----------------
thetheodor wrote:
> One potential issue with having an A0 datalayout and addrspace 5 allocas
> could be that various passes (including SDAG Legalization) could insert
> addrspace 0 allocas based on the datalayout while our current lowering would
> expect them to be in addrspace 5.
I already found such a case when lowering `extract_vector_elt`, I fixed it with
[this
change](https://github.com/llvm/llvm-project/pull/154814/files#diff-dd5fa182d7db05b7e71d6ca1be1b711d28153d6d6559609d81a9457dcf5ad0daR1083)
but I'm not very confident that there are no other such cases not covered by
existing tests.
I would lean against changing the DL only in the modules as I had to make a few
dubious changes to make this work (treating `-A0` and `-A5` as "compatible" in
`NVPTX::isCompatibleDataLayout` and doing the same in clang which compares the
TM's and Module's DLs).
Regarding changing the TM DL: I believe it's cleaner/safer in the sense that
inserted allocas should automatically have correct address space (and any
leftovers will be handled by the pass) and there's no need to special case
anything that might accidentally pass through/be created after the lowering.
Two main concerns are:
1. Major changes in the OpenMP tests, but as mentioned above, these would
converge to other gpu targets (e.g., AMDGPU).
2. This is potentially a drastic change with potentially unforeseen
implications especially for out-of-tree users. I will evaluate with on our
internal benchmarks to make sure there are no surprises, and I'd be also happy
to test this with other available benchmarks (@Artem-B any recommendations?).
https://github.com/llvm/llvm-project/pull/154814
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
