Page MenuHomePhabricator

[ARM] Add missing memory operands to a bunch of instructions.
ClosedPublic

Authored by efriedma on Mar 22 2019, 1:05 PM.

Details

Summary

This should hopefully lead to minor improvements in code generation, and more accurate spill/reload comments in assembly.

Diff Detail

Repository
rL LLVM

Event Timeline

efriedma created this revision.Mar 22 2019, 1:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2019, 1:05 PM
SjoerdMeijer accepted this revision.Mar 23 2019, 2:48 AM

LGTM, just a question inline.

lib/Target/ARM/ARMBaseInstrInfo.cpp
1180 ↗(On Diff #191931)

I can see that this extra check makes sense, just curious how it relates to the other changes.

This revision is now accepted and ready to land.Mar 23 2019, 2:48 AM
efriedma marked 2 inline comments as done.Mar 25 2019, 12:26 PM
efriedma added inline comments.
lib/Target/ARM/ARMBaseInstrInfo.cpp
1180 ↗(On Diff #191931)

Without this, the assembly remarks get messed up; instead of "@ 8-byte Folded Spill", we get something like "@ 4-byte Spill" for an instruction that accesses two spill slots.

efriedma marked an inline comment as done.Mar 25 2019, 12:27 PM
This revision was automatically updated to reflect the committed changes.

This broke compilation for me in a number of cases, see https://bugs.llvm.org/show_bug.cgi?id=41231 for repro.

This broke compilation for me in a number of cases, see https://bugs.llvm.org/show_bug.cgi?id=41231 for repro.

There is also a similar error message appearing in the stage-2 of the self-hosting v7 builder clang-cmake-armv7-selfhost-neon and clang-cmake-armv7-selfhost http://lab.llvm.org:8011/builders/clang-cmake-armv7-selfhost/builds/1607/steps/build%20stage%202/logs/stdio

FAILED: /home/buildslave/buildslave/clang-cmake-armv7-selfhost/stage1.install/bin/clang++   -DGTEST_HAS_RTTI=0 -D_DEBUG -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/CodeGen/GlobalISel -I/home/buildslave/buildslave/clang-cmake-armv7-selfhost/llvm/lib/CodeGen/GlobalISel -I/usr/include/libxml2 -Iinclude -I/home/buildslave/buildslave/clang-cmake-armv7-selfhost/llvm/include -mcpu=cortex-a15 -mfpu=vfpv3 -marm -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3    -UNDEBUG  -fno-exceptions -fno-rtti -MMD -MT lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/Utils.cpp.o -MF lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/Utils.cpp.o.d -o lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/Utils.cpp.o -c /home/buildslave/buildslave/clang-cmake-armv7-selfhost/llvm/lib/CodeGen/GlobalISel/Utils.cpp
fatal error: error in backend: Expected a variant SchedClass
clang-9: error: clang frontend command failed with exit code 70 (use -v to see invocation)
clang version 9.0.0 (trunk 356970)
Target: armv7l-unknown-linux-gnueabihf
Thread model: posix
InstalledDir: /home/buildslave/buildslave/clang-cmake-armv7-selfhost/stage1.install/bin
clang-9: note: diagnostic msg: PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
clang-9: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-9: note: diagnostic msg: /tmp/Utils-6cbb63.cpp
clang-9: note: diagnostic msg: /tmp/Utils-6cbb63.sh
clang-9: note: diagnostic msg: 

********************

It does not appear to be showing up on our Arm v8 builders clang-cmake-armv8-selfhost-neon , and I've not yet been able to reproduce that on a v8 machine. One difference is that the v7 builders use gcc 5.4 for stage-1 whereas I think the v8 builders use clang. I will try and reproduce with gcc 5.4.0 on a v8 machine.

It does not appear to be showing up on our Arm v8 builders clang-cmake-armv8-selfhost-neon , and I've not yet been able to reproduce that on a v8 machine. One difference is that the v7 builders use gcc 5.4 for stage-1 whereas I think the v8 builders use clang. I will try and reproduce with gcc 5.4.0 on a v8 machine.

I've been able to reproduce the buildbot failure on an x86 machine. It requires --target=armv7a-linux-gnueabihf -mcpu=cortex-a15 on CodeGen/GlobalISel/Utils.cpp. On our v8 builder -mcpu=cortex-a57 is used so this is why we didn't see the crash. From a build directory alongside LLVM

clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/CodeGen/GlobalISel -I../llvm/lib/CodeGen/GlobalISel -Iinclude -I../llvm/include --target=armv7a-linux-gnueabihf -mcpu=cortex-a15 -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -UNDEBUG -fno-exceptions -fno-rtti -MMD -MT lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/Utils.cpp.o -MF lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/Utils.cpp.o.d -o lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/Utils.cpp.o -c ../llvm/lib/CodeGen/GlobalISel/Utils.cpp

The reproducer in the PR is much shorter though.