assigned to @aemerson

Somewhat reduced repro steps from justincohen:

1. Create Sample Xcode project. I'm using 10.2 but it doesn't seem to matter.
2. Rename ViewController.m -> ViewController.mm
3. Disable project setting COMPILER_INDEX_STORE_ENABLE and add user defined settings CC and CXX pointing to CLANG_REVISION = '353250'
   buildSettings {
   CC = "/path/to/src/third_party/llvm-build/Release+Asserts/bin/clang";
   CXX = "/path/to/src/third_party/llvm-build/Release+Asserts/bin/clang";
   }
4. Use the following sample code for ViewController.mm

#import "ViewController.h"

@​interface ViewController ()
@​end

@​implementation ViewController

• (void)bar {
  for (int a = 0; a<10; a++) {
  NSLog(@"bar");
  }
  }

• (void)foo {
  int b = 5;
  for (int a = 0; a<10; a++) {
  NSLog(@"foo");
  [self bar];
  b++;
  }
  }

• (void)viewDidLoad {
  [super viewDidLoad];
  [self foo];
  // Do any additional setup after loading the view.
  }

@​end

5. Build and run on a device. I'm using an iPhone X but it doesn't seem to matter.
6. Set a breakpoint at super viewDidLoad];and attempt to step into each method and over each line.
7. Note that the instruction pointer jumps all over the place on each step.

Update from the other bug:

• -fno-experimental-isel makes this go away, so it's due to global isel
• but the issue seems present at 348319 too, so likely not due to r348320

If there's a particular function/method that is "jumpy" it could help
to post a source and corresponding .o file, and describe what source
lines seem to have the problem. Then we could look at the line table
compared to the source and see if anything recognizable pops out.

jumpy ViewController.o
Here's the .o file with the jumpy behavior

good ViewController.o, built with -fno-experimental-isel
And here's a working version, built without global isel

(Source is in comment 1)

And the compile line Xcode used for that is

/path/to/work/llvm-project/out/gn/bin/clang -x objective-c++ -arch arm64 -fmessage-length=0 -fdiagnostics-show-note-include-stack -fmacro-backtrace-limit=0 -std=gnu++14 -stdlib=libc++ -fobjc-arc -fobjc-weak -fmodules -gmodules -fmodules-cache-path=/path/to/DerivedData/ModuleCache.noindex -fmodules-prune-interval=86400 -fmodules-prune-after=345600 -fbuild-session-file=/path/to/DerivedData/ModuleCache.noindex/Session.modulevalidation -fmodules-validate-once-per-build-session -Wnon-modular-include-in-framework-module -Werror=non-modular-include-in-framework-module -Wno-trigraphs -fpascal-strings -O0 -fno-common -Wno-missing-field-initializers -Wno-missing-prototypes -Werror=return-type -Wdocumentation -Wunreachable-code -Wno-implicit-atomic-properties -Werror=deprecated-objc-isa-usage -Wno-objc-interface-ivars -Werror=objc-root-class -Wno-arc-repeated-use-of-weak -Wimplicit-retain-self -Wno-non-virtual-dtor -Wno-overloaded-virtual -Wno-exit-time-destructors -Wduplicate-method-match -Wno-missing-braces -Wparentheses -Wswitch -Wunused-function -Wno-unused-label -Wno-unused-parameter -Wunused-variable -Wunused-value -Wempty-body -Wuninitialized -Wconditional-uninitialized -Wno-unknown-pragmas -Wno-shadow -Wno-four-char-constants -Wno-conversion -Wconstant-conversion -Wint-conversion -Wbool-conversion -Wenum-conversion -Wno-float-conversion -Wnon-literal-null-conversion -Wobjc-literal-conversion -Wshorten-64-to-32 -Wno-newline-eof -Wno-selector -Wno-strict-selector-match -Wundeclared-selector -Wdeprecated-implementations -Wno-c++11-extensions -DDEBUG=1 -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS12.2.sdk -fstrict-aliasing -Wprotocol -Wdeprecated-declarations -Winvalid-offsetof -miphoneos-version-min=12.0 -g -fvisibility-inlines-hidden -Wno-sign-conversion -Winfinite-recursion -Wmove -Wcomma -Wblock-capture-autoreleasing -Wstrict-prototypes -Wrange-loop-analysis -Wno-semicolon-before-method-body -Wunguarded-availability -fembed-bitcode-marker -iquote /path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/test_case-generated-files.hmap -I/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/test_case-own-target-headers.hmap -I/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/test_case-all-target-headers.hmap -iquote /path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/test_case-project-headers.hmap -I/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Products/Debug-iphoneos/include -I/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/DerivedSources-normal/arm64 -I/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/DerivedSources/arm64 -I/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/DerivedSources -F/path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Products/Debug-iphoneos -MMD -MT dependencies -MF /path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/Objects-normal/arm64/ViewController.d --serialize-diagnostics /path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/Objects-normal/arm64/ViewController.dia -c /path/to/work/test_case/test_case/ViewController.mm -o /path/to/DerivedData/test_case-cbdhovakpwavafefdumbsossfcgm/Build/Intermediates.noindex/test_case.build/Debug-iphoneos/test_case.build/Objects-normal/arm64/ViewController.o

(optionally add -fno-experimental-isel)

Commits like r348320 reduce the fallback rate of GlobalISel, resulting in more functions being successfully compiled with GISel vs FastISel/SelectionDAG. This might expose existing underlying issues even if the commit is correct. I'm on vacation but we'll take a look as soon as we can.

FYI, I tried reverting Wed Dec 5 [AArch64][GlobalISel] Re-enable selection of volatile loads with my sample, and still see jumpy debug info. This is with experimental-isel on.

Do you think you could attach the .dSYMs for the good and bad binaries?

The object files don't contain enough information to actually root cause this.

Created attachment 21519 [details]
good ViewController.o, built with -fno-experimental-isel

And here's a working version, built without global isel

no experimental_isel dsym + objects
recompiled with experimental isel off ViewController.mm, with all object files and dsyms.

experimental isel on
recompiled with experimental isel on w/ ViewController.mm, with all object files and dsyms.

jpaquette, do these help?

jpaquette, ping?

I think I've hit the same problem on arm64 Android.

Example:

int var1; // 1
int var2; // 2
int main() { // 3
if (var1 == 1) { // 4
var2 = 2; // 5
} // 6
return 0; // 7
}

On Ubuntu 16.04 ARM64, build using: ~/clang+llvm-8.0.0-aarch64-linux-gnu/bin/clang test.c -O0 -g

Stepping through this program in gdb steps through lines: 4 -> 5 -> 4 -> 7. A breakpoint on line 5 is hit.

With -fno-experimental-isel, gdb steps through lines 4 -> 7 only, and a breakpoint on line 5 isn't hit.

Clang appears to move instructions loading a symbol's address to the start of a function, but it still attributes the instructions to the line where they came from:

main:
.loc 1 3 0 // test.c:3:0
sub sp, sp, #​16

.loc 1 4 7 prologue_end // test.c:4:7
adrp x8, var1
add x8, x8, :lo12:var1

.loc 1 4 12 is_stmt 0 // test.c:4:12
orr w9, wzr, #​0x1

.loc 1 5 10 is_stmt 1 // test.c:5:10
adrp x10, var2
add x10, x10, :lo12:var2
str wzr, [sp, #​12]
^^^^^^^^^^^--- I think marking these instructions as line 5 is a problem?

.loc 1 4 7 // test.c:4:7
ldr w11, [x8]

.loc 1 4 12 is_stmt 0 // test.c:4:12
cmp w11, w9
cset w9, eq
str x10, [sp]

.loc 1 4 7 // test.c:4:7
tbnz w9, #​0, .LBB0_1
b .LBB0_2
.LBB0_1:

.loc 1 5 10 is_stmt 1 // test.c:5:10
orr w8, wzr, #​0x2
ldr x9, [sp]
str w8, [x9]
.LBB0_2:

.loc 1 0 10 is_stmt 0 // test.c:0:10
mov w0, #​0

.loc 1 7 3 is_stmt 1 // test.c:7:3
add sp, sp, #​16
ret

I see the same line annotations with "-target arm64-apple-darwin11". Passing -fno-experimental-isel appears to fix the issue. With that flag, the load of var2's address happens later in the function and is attributed to line 0 instead:

cmp w9, #​1
b.ne .LBB0_2

.loc 1 0 7 // test.c:0:7
adrp x8, var2
add x8, x8, :lo12:var2

.loc 1 5 10 is_stmt 1 // test.c:5:10
orr w9, wzr, #​0x2
str w9, [x8]
.LBB0_2:

I'll take over this. Thanks for the further analysis Ryan, that's very helpful.

I think we should be setting a line number of 0 for these instructions which are hoisted to other blocks, as LICM does.

Regardless, we should probably not be hoisting the var2 ADRP at all, there's no CSE re-use going on and it's just unnecessarily increasing register pressure.

For reference, I opened an Android NDK issue, android/ndk#1004.

Patch in review: https://reviews.llvm.org/D63286

Should be fixed as of r363331.