Hi,
On 2021-04-22 09:35:48 -0700, Tom Stellard wrote:
> On 4/21/21 6:40 AM, Honza Horak wrote:
> I wrote a new patch based on the bug discussion[1]. It works around
> the issue specifically on s390x rather than disabling specific
> CPUs and features for all targets. The patch is attached.
Cool, this is a pretty clear improvement. There's a few minor things I'd
change to fit it into PG - do you mind if I send that to the thread at
[1] for you to test before I push it?
> +/*
> + * For the systemz target, LLVM uses a different datalayout for z13 and newer
> + * CPUs than it does for older CPUs. This can cause a mismatch in
> datalayouts
> + * in the case where the llvm_types_module is compiled with a pre-z13 CPU
> + * and the JIT is running on z13 or newer.
> + * See computeDataLayout() function in
> + * llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp for information on the
> + * datalayout differences.
> + */
> +static bool
> +needs_systemz_workaround(void)
> +{
> + bool ret = false;
> + LLVMContextRef llvm_context;
> + LLVMTypeRef vec_type;
> + LLVMTargetDataRef llvm_layoutref;
> + if (strncmp(LLVMGetTargetName(llvm_targetref), "systemz",
> strlen("systemz")))
> + {
> + return false;
> + }
> +
> + llvm_context = LLVMGetModuleContext(llvm_types_module);
> + vec_type = LLVMVectorType(LLVMIntTypeInContext(llvm_context, 32), 4);
> + llvm_layoutref = LLVMCreateTargetData(llvm_layout);
> + ret = (LLVMABIAlignmentOfType(llvm_layoutref, vec_type) == 16);
> + LLVMDisposeTargetData(llvm_layoutref);
> + return ret;
> +}
I wonder if it'd be better to compare LLVMCopyStringRepOfTargetData() of
the llvm_types_module with the one of the JIT target machine, and only
specify -vector in that case? We currently support older LLVM versions
than the one that introduced the vector specific handling for systemz,
and I don't know what'd happen if we unnecessarily specified -vector.
Greetings,
Andres Freund
