[mlir][SystemZ] Disable `-fno-semantic-interposition` option.

[imaihal]

Summary

-fno-semantic-interposition was added for GCC in D102453, but MLIR test
on SystemZ failed when GCC version is less than 10.3 because of GCC bug.
This patch disables the option only on SystemZ when using GCC version less than 10.3.

Signed-off-by: Haruki Imai <[e-mail]>

[imaihal]

After D102453, MLIR installation test (cmake --build . --target check-mlir) issues 32 errors on SystemZ with GCC. This errors can be avoided by disabling fno-semantic-interposition. I used g++ (Ubuntu 9.4.0-1ubuntu1~18.04) 9.4.0.

One of the errors is as follows. Also in other errors, operands seems to be broken.

llvm-project/mlir/test/Conversion/GPUCommon/lower-memcpy-to-gpu-runtime-calls.mlir:13:11: error: 'llvm.getelementptr' op expected 1 or more operands
    %t1 = gpu.memcpy async [%t0] %dst, %src : memref<7xf32, 1>, memref<7xf32>

Following is generated inc file (LLVMOps.cpp.inc). I think the operands is broken, but when I inserted printf here, the error disappears.

void GEPOp::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Type resultType, ValueRange operands, ArrayRef<NamedAttribute> attributes ) {
    if (resultType) odsState.addTypes(resultType);
    odsState.addOperands(operands);
    for (auto namedAttr : attributes) {
      odsState.addAttribute(namedAttr.first, namedAttr.second);
    }
  
}

Hi, @MaskRay This patch modifies your patch (D102453). Do you have any suggestions or comments?

[MaskRay]

In D105453#2859200, @imaihal wrote:

After D102453, MLIR installation test (cmake --build . --target check-mlir) issues 32 errors on SystemZ with GCC. This errors can be avoided by disabling fno-semantic-interposition. I used g++ (Ubuntu 9.4.0-1ubuntu1~18.04) 9.4.0.

One of the errors is as follows. Also in other errors, operands seems to be broken.

llvm-project/mlir/test/Conversion/GPUCommon/lower-memcpy-to-gpu-runtime-calls.mlir:13:11: error: 'llvm.getelementptr' op expected 1 or more operands
    %t1 = gpu.memcpy async [%t0] %dst, %src : memref<7xf32, 1>, memref<7xf32>

Following is generated inc file (LLVMOps.cpp.inc). I think the operands is broken, but when I inserted printf here, the error disappears.

void GEPOp::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Type resultType, ValueRange operands, ArrayRef<NamedAttribute> attributes ) {
    if (resultType) odsState.addTypes(resultType);
    odsState.addOperands(operands);
    for (auto namedAttr : attributes) {
      odsState.addAttribute(namedAttr.first, namedAttr.second);
    }
  
}

Hi, @MaskRay This patch modifies your patch (D102453). Do you have any suggestions or comments?

I don't mind. I suspect there is a gcc SystemZ bug but I don't have access to a SystemZ machine.
Worth filing a gcc bug if you can spend more time on investigating the issue.

[imaihal]

Condition to disable the option was updated since we found GCC version >= 11 is no problem.

[imaihal]

In D105453#2860280, @MaskRay wrote:

I don't mind. I suspect there is a gcc SystemZ bug but I don't have access to a SystemZ machine.
Worth filing a gcc bug if you can spend more time on investigating the issue.

@MaskRay Thanks for your comments. We have not found the actual bug yet, but it seems it was solved in GCC version >= 11. So I updated the condition in this patch.

@MaskRay @phosek Is it possible to be a reviewer of this patch? I asked because you were a developer and a reviewer of D102453.

[imaihal]

Updated the condition to disable the option. It is disabled when GCC version is less than 10.3.

[MaskRay]

It'd be good to know which gcc bug this is.

Perhaps just exclude SystemZ for all configurations, instead of checking "mlir" IN_LIST LLVM_ENABLE_PROJECTS. Include a link to the bug report.

We generally still want -fno-semantic-interposition because gcc -fno-semantic-interposition built clang can be 3% faster on some benchmarks.

[imaihal]

Removed checking for MLIR build. Now the option is disabled for all configuration of SystemZ.

[imaihal]

@MaskRay Thanks for comments.

Perhaps just exclude SystemZ for all configurations, instead of checking "mlir" IN_LIST LLVM_ENABLE_PROJECTS.

I removed the check.

Include a link to the bug report.

We are investigating the bug report.

Thanks.

[MaskRay]

Looks great!

llvm/cmake/modules/HandleLLVMOptions.cmake

321–326

GCC<11 has bugs on SystemZ, so don't use the option for older GCC.

[MaskRay]

Oh, new GCC works? That's great.

[imaihal]

Updated the comment. We confirmed GCC version >= 10.3 works, not >= 11.

[imaihal]

Updated the comment again to reflect @MaskRay`s comment.

[imaihal]

@MaskRay Yes. New GCC (>= 10.3) works.

Don't I need to include the bug report? It seems it takes much time to identify which bug fix(bug report) solved this issue..
If so, could you land this patch? I don't have commit access.

[MaskRay]

In D105453#2866218, @imaihal wrote:

@MaskRay Yes. New GCC (>= 10.3) works.

Don't I need to include the bug report? It seems it takes much time to identify which bug fix(bug report) solved this issue..
If so, could you land this patch? I don't have commit access.

Not required. I initially thought this affects latest GCC, then having a bug number will help us to track the progress and ensure the workaround can be removed in time.

Since this is actually a problem fixed in latest versions, a bug reference is less important. But if you can identify the issue it'd be good to add a note as well, but not strictly required.

I'll commit it on your behalf

llvm/cmake/modules/HandleLLVMOptions.cmake

else()
     add_flag_or_print_warning("-fPIC" FPIC)
     # Enable interprocedural optimizations for non-inline functions which would
     # otherwise be disabled due to GCC -fPIC's default.
     #
     # Note: Clang allows IPO for -fPIC so this optimization is less effective.
     # Older Clang may support -fno-semantic-interposition but it used local
     # aliases to optimize global variables, which is incompatible with copy
     # relocations due to -fno-pic.
-    if (CMAKE_COMPILER_IS_GNUCXX OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND
-        CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL 13))
+    # GCC for SystemZ: Enable `-fno-semantic-interposition` when using
+    # GCC version >= 10.3 Old versions of GCC seems to have bugs.
+    if ((CMAKE_COMPILER_IS_GNUCXX AND
+    NOT (LLVM_NATIVE_ARCH STREQUAL "SystemZ"
+      AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 10.3))
+  OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL 13))
       add_flag_if_supported("-fno-semantic-interposition" FNO_SEMANTIC_INTERPOSITION)
     endif()
   endif()
   # GCC for MIPS can miscompile LLVM due to PR37701.
   if(CMAKE_COMPILER_IS_GNUCXX AND LLVM_NATIVE_ARCH STREQUAL "Mips" AND
          NOT Uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
     add_flag_or_print_warning("-fno-shrink-wrap" FNO_SHRINK_WRAP)
   endif()