Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Apple M1: bug in jitting argument of type short #7090

Closed
Axel-Naumann opened this issue Jan 25, 2021 · 5 comments · Fixed by #7787
Closed

Apple M1: bug in jitting argument of type short #7090

Axel-Naumann opened this issue Jan 25, 2021 · 5 comments · Fixed by #7787
Assignees
@hahnjo
Labels
bug in:Cling
Projects
Fixed in 6.24/00 Fixed in 6.26/00 Fixed in 6.22/10
Milestone
6.24/00

Comments

@Axel-Naumann
Copy link
Member

Axel-Naumann commented Jan 25, 2021 •
edited by couet
Loading

Describe the bug

new TPad("p","p",0.,0.,1.,1.) on the prompt (or anything jitted by the interpreter) will pass 65535 for color even though the constructor is declared as TPad (const char *name, const char *title, Double_t xlow, Double_t ylow, Double_t xup, Double_t yup, Color_t color=-1, Short_t bordersize=-1, Short_t bordermode=-2) where Color_t is short.

Apparently, cling and the compiled library disagree on the size of short?

Expected behavior

TPad() should see color == -1 being passed, as per the default argument.

To Reproduce

new TPad("p","p",0.,0.,1.,1.) shows the wrong background color.

Setup

Apple M1, ROOT master built with clang.
@Axel-Naumann Axel-Naumann self-assigned this Jan 25, 2021
@github-actions github-actions bot added this to Needs triage in Triage Jan 25, 2021
@couet
Copy link
Member

couet commented Jan 27, 2021

This bug makes stressGraphics fail on Apple M1 machines.

@eguiraud eguiraud removed this from Needs triage in Triage Feb 10, 2021
@jalopezg-git
Copy link
Collaborator

jalopezg-git commented Mar 30, 2021 •
edited
Loading

65535 is the ones-complement representation of -1. It seems that, in some contexts, a short is internally converted to some unsigned type internally. This issue is not specific to functions with arguments using default values, e.g., one could reproduce with

root [0] std::cout << (short)-1 << std::endl;
65535

Taking a deeper look into it now.

@Axel-Naumann Axel-Naumann assigned hahnjo and unassigned jalopezg-git Mar 31, 2021
@hahnjo
Copy link
Member

hahnjo commented Apr 1, 2021

Want to see something funny? I had a ROOT build in Debug mode laying around on macphsft25, added a std::cout to print the color value in TPad::TPad - and was very surprised to see -1. Rebuilt in Release mode and now it's 65535 🤔

@Axel-Naumann
Copy link
Member Author

Hmm that would hint at an ABI issue - and that should be reproducible with lli, even though it might need a JIT -> compiled library call to expose the ABI incompatibility. FYI and in case you don't know, you can load a .so into lli with -load= - that's not what the argument was invented for, but it works like a charm :-)

@hahnjo hahnjo changed the title Apple M1: bug in jitting default argument of type short Apple M1: bug in jitting argument of type short Apr 6, 2021
hahnjo added a commit to hahnjo/root that referenced this issue Apr 6, 2021
@hahnjo
[cling] Set full target for Apple M1
eb51ff4 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes root-project#7090
@hahnjo hahnjo added this to the 6.24/00 milestone Apr 6, 2021
@hahnjo hahnjo mentioned this issue Apr 6, 2021
Merged
@hahnjo
Copy link
Member

hahnjo commented Apr 6, 2021

Yep, it's an ABI issue: On AArch64, integers with less than 32 bits (including shorts) are promoted to 32 bits - so far, so good, everybody agrees on that. Where things start to differ is how to do that, Darwin expects extension whereas the generic method is "direct". I suppose the latter means filling with zeros in practice, so (short)-1 = 0xffff -> 0x0000ffff = 65535. Setting up the target triple in a different way in #7787 allows Clang's Driver logic to compute the ABI correctly.

hahnjo added a commit that referenced this issue Apr 6, 2021
@hahnjo
[cling] Set full target for Apple M1 (#7787)
f75a7c4 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes #7090
hahnjo added a commit to hahnjo/root that referenced this issue Apr 6, 2021
@hahnjo
[cling] Set full target for Apple M1 (root-project#7787)
26715e4 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes root-project#7090

(cherry picked from commit f75a7c4)
hahnjo added a commit to hahnjo/root that referenced this issue Apr 6, 2021
@hahnjo
[cling] Set full target for Apple M1 (root-project#7787)
8e3c25d 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes root-project#7090

(cherry picked from commit f75a7c4)
hahnjo added a commit to hahnjo/root that referenced this issue Apr 6, 2021
@hahnjo
[cling] Set full target for Apple M1
b447dc9 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes root-project#7090

(cherry picked from commit f75a7c4)
hahnjo added a commit to hahnjo/root that referenced this issue Apr 6, 2021
@hahnjo
[cling] Set full target for Apple M1
d5e8c8d 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes root-project#7090

(cherry picked from commit f75a7c4)
This was referenced Apr 6, 2021
Merged
Merged
hahnjo added a commit that referenced this issue Apr 7, 2021
@hahnjo
[cling] Set full target for Apple M1 (#7792)
82a8e71 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes #7090

(cherry picked from commit f75a7c4)
@hahnjo hahnjo added this to Issues in Fixed in 6.24/00 via automation Apr 7, 2021
@hahnjo hahnjo added this to Issues in Fixed in 6.26/00 via automation Apr 7, 2021
hahnjo added a commit that referenced this issue Apr 7, 2021
@hahnjo
[cling] Set full target for Apple M1 (#7793)
2f09433 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes #7090

(cherry picked from commit f75a7c4)
@hahnjo hahnjo added this to Fixed in Fixed in 6.22/10 via automation Apr 7, 2021
mrodozov pushed a commit to cms-sw/root that referenced this issue Apr 29, 2021
@hahnjo @mrodozov
[cling] Set full target for Apple M1 (root-project#7793)
4d12fe3 
The argument -Xclang -triple=... completely bypasses Clang's Driver
logic and only sets the triple in CC1. This suffices for most code
generation tasks, but the Driver cannot compute the correct ABI and
sets the generic AArch64 "aapcs" instead of the specific "darwinpcs".
In turn, this causes integer arguments with less than 32 bits not
being sign-extended but being passed directly, which for example
manifests as (short)-1 being read as 65535 on the callee side.

The new argument --target=arm64-apple-darwin20.3.0 matches what
Apple's and LLVM main's clang return for --print-target-triple.

Fixes root-project#7090

(cherry picked from commit f75a7c4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hahnjo hahnjo

Labels
bug in:Cling
Projects
Fixed in 6.22/10
Fixed
Fixed in 6.24/00
Issues
Fixed in 6.26/00
Issues
Milestone
6.24/00
Development

Successfully merging a pull request may close this issue.

[cling] Set full target for Apple M1 hahnjo/root
4 participants
@Axel-Naumann @hahnjo @couet @jalopezg-git