Were the changes to arm-pcsections.ll in https://reviews.llvm.org/rGbc7d88faf1a595ab59952a2054418cdd0d9eeee8#change-knDnIXP6oBYw intended? We're seeing basically every single one of those fail when running the test locally. I'm also seeing bots failing, e.g. https://lab.llvm.org/buildbot/#/builders/119/builds/15375
Implement review feedback from Orlando.
static_assert with no message requires --std=c++17 which is not present for this test object. Otherwise we could add that here. The build command for that object is currently:COMMAND = cmd.exe /C "cd /D C:\git\llvm-project\stage1_msvc\projects\compiler-rt\lib\asan\tests && C:\git\llvm-project\stage1_msvc\.\bin\clang.exe -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -IC:/git/llvm-project/llvm/../third-party/unittest/googletest/include -IC:/git/llvm-project/llvm/../third-party/unittest/googletest -Wno-deprecated-declarations -IC:/git/llvm-project/compiler-rt/include -IC:/git/llvm-project/compiler-rt/lib -IC:/git/llvm-project/compiler-rt/lib/asan -IC:/git/llvm-project/compiler-rt/lib/sanitizer_common/tests -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS -fno-rtti -O2 -Wno-format -Werror=sign-compare -gline-tables-only -gcodeview -DASAN_HAS_IGNORELIST=1 -DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -c -o ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cpp.x86_64-inline.o C:/git/llvm-project/compiler-rt/lib/asan/tests/asan_noinst_test.cpp"
FYI it looks like this is causing crashes on ARM64.
we're seeing crashes around RegisterCoalescer. I haven't bisected exactly which patch yet but I figured it's one of these patches
This change is problematic, as something like
if (x >= '0' && x <= '9')
before this change is "if (x - '0‘ <u 10)", and now it becomes "if (245 <u x - '9')".
The later one is more expensive, as the ZExt followed by ADD(of type i32) is not removable, say, if the ISA does not have native i8-ADD instruction. While the former form is just OK, and the ZExt is removable.
So at least, if the type does not have native addition instruction, we should not do this. Or maybe, just don't do this in general, as no cannonicalization in that sense is possible.
Addressed the comments so far.
Updated the patch, thank you.