x86 hits assertion ModRM Fields out of range!

[ye-luo]

Checked today e33867a
I only make one build every night. The failure happened between e995526 (good) and 1889170 (bad)

clang++: /..../test_llvm/llvm-project/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:104: uint8_t modRMByte(unsigned int, unsigned int, unsigned int): Assertion `Mod < 4 && RegOpcode < 8 && RM < 8 && "ModRM Fields out of range!"' failed.

full back trace detail in README in the zip bug_report_x86.zip

[llvmbot]

@llvm/issue-subscribers-backend-x86

[topperc]

Can you provide the 2 files listed at the bottom of the crash

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-15: note: diagnostic msg: /tmp/multi_bspline_eval_z_std3-782f99-15d500.cpp
clang-15: note: diagnostic msg: /tmp/multi_bspline_eval_z_std3-782f99-15d500.sh
clang-15: note: diagnostic msg: 

[nicolas17]

I can already reproduce this with clang++ -c -march=skylake -Drestrict=__restrict__ -O3 -std=c++17 and the .cpp file in the zip. Bisecting...

[ye-luo]

Bisect shows failure starts with 702d5de

In addition, when I was checking 1889170, I noticed the following.
As long as I build clang with clang from llvm release (tested 11.0.1, 11.1.0, 13.0.0, 14.0.1) assertion stops the compilation.
Once I build clang with gcc 9.4. The test case can be compiled successfully.

here is my clang build recipe.

cmake -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=$INSTALL_FOLDER \
    -DLLVM_ENABLE_BACKTRACES=ON \
    -DLLVM_ENABLE_WERROR=OFF \
    -DBUILD_SHARED_LIBS=OFF \
    -DLLVM_ENABLE_RTTI=ON \
    -DLLVM_TARGETS_TO_BUILD="X86;AMDGPU;NVPTX" \
    -DLLVM_ENABLE_ASSERTIONS=ON \
    -DLLVM_ENABLE_PROJECTS=clang \
    ../llvm-project/llvm

[nicolas17]

702d5de4380b1e1554e5b90863093c3a57f76f70 is the first bad commit
commit 702d5de4380b1e1554e5b90863093c3a57f76f70
Author: Nikita Popov
Date:   Thu Apr 7 11:59:38 2022 +0200

    [Clang] Enable opaque pointers by default
    
    Enable opaque pointers by default in clang, which can be disabled
    either via cc1 option -no-opaque-pointers or cmake flag
    -DCLANG_ENABLE_OPAQUE_POINTERS=OFF.
    
    See https://llvm.org/docs/OpaquePointers.html for context.
    
    Differential Revision: https://reviews.llvm.org/D123300

 clang/CMakeLists.txt         | 2 +-
 llvm/docs/OpaquePointers.rst | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

EDIT: bah beaten by a minute :)

[EugeneZelenko]

@nikic

[nicolas17]

Testing the "good" commit e995526 with -Xclang -opaque-pointers fails with the assertion, and testing the "bad" commit 1889170 with -Xclang -no-opaque-pointers doesn't fail, so this bisect doesn't help much beyond "it's because of opaque pointers"...

[KanRobert]

Small reproducer:

$ cat reduced.ll

%struct.widget = type { { double, double } }

define void @baz(ptr %arg, <4 x i1> %arg1) #0 {
bb:
  %tmp = shufflevector <2 x double> zeroinitializer, <2 x double> zeroinitializer, <4 x i32> zeroinitializer
  br label %bb2

bb2:                                              ; preds = %bb2, %bb
  %tmp3 = fmul <4 x double> zeroinitializer, zeroinitializer
  %tmp4 = getelementptr inbounds %struct.widget, ptr %arg, <4 x i64> undef
  %tmp5 = call <4 x double> @llvm.masked.gather.v4f64.v4p0(<4 x ptr> %tmp4, i32 0, <4 x i1> %arg1, <4 x double> zeroinitializer)
  %tmp6 = fadd <4 x double> zeroinitializer, zeroinitializer
  %tmp7 = extractelement <4 x double> %tmp5, i64 0
  store double %tmp7, ptr null, align 8
  br label %bb2
}

declare <4 x double> @llvm.masked.gather.v4f64.v4p0(<4 x ptr>, i32 immarg, <4 x i1>, <4 x double>)

attributes #0 = { "target-cpu"="skylake" }

$ llc --filetype=obj reduced.ll

[phoebewang]

I think it is the same root cause as #55021

[KanRobert]

From the result of -print-after-all

An illegal MIR is generated

early-clobber %6:vr256, early-clobber %7:vr256 = VGATHERQPDYrm %3:vr256(tied-def 0), %0:gr64, 16, killed %8:vr256, 0, $noreg, %5:vr256(tied-def 1) :: (load unknown-size, align 8)

The scalar field of the memory operand is set to 16, which should be one of 1, 2, 4, 8.

[phoebewang]

The 16 comes from type { { double, double } } which generates an invalid index to gather instruction. But I think we may need to add the check in machine verifier to error out earlier.

[KanRobert]

The 16 comes from type { { double, double } } which generates an invalid index to gather instruction. But I think we may need to add the check in machine verifier to error out earlier.

Good suggestion. Created https://reviews.llvm.org/D124455 to crash after ISEL.

[nikic]

Fixed by 027c728.