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.

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

reg 1.31 : variable used in GEP cause other constant to be threaded as if equal to 0 #4362

Closed
ghost opened this issue Apr 1, 2023 · 5 comments · Fixed by #4365
Closed

reg 1.31 : variable used in GEP cause other constant to be threaded as if equal to 0 #4362

ghost opened this issue Apr 1, 2023 · 5 comments · Fixed by #4365

Comments

@ghost
Copy link

ghost commented Apr 1, 2023

Note sure about the title however see https://godbolt.org/z/ePxshnvvc.

In the second case, the same pointer is displayed twice, unless you add the const storage class to RR.

@ghost ghost changed the title reg 1.31 : variable used in GEP threated like equal to 0 reg 1.31 : variable used in GEP threated as if equal to 0 Apr 1, 2023
@ghost ghost changed the title reg 1.31 : variable used in GEP threated as if equal to 0 reg 1.31 : variable used in GEP cause other constant to be threaded as if equal to 0 Apr 2, 2023
@ghost
Copy link
Author

ghost commented Apr 2, 2023

Better test case:

void main()
{
    int[2][1] arr;
    assert(&(arr[0][0]) !is &(arr[0][1]));
    ubyte RR = 0;
    assert(&(arr[RR][0]) !is &(arr[RR][1])); // fails
}     

can that be caused by the switch to untyped pointers ?

@JohanEngelen
Copy link
Member

Link to godbolt with AST and LLVM IR output: https://godbolt.org/z/aajE5nKK3

The ASTs are different:

// LDC 1.30
assert(&arr[cast(ulong)RR][0] !is &arr[cast(ulong)RR][1]);

// LDC 1.31
assert(&arr[cast(ulong)RR] + 0LU !is &arr[cast(ulong)RR] + 4LU);

The new offset addition + x (that replaced the [] array indexing) is apparently ignored by LDC in 1.31, as it is not translated into LLVM IR:

; LDC 1.30
%19 = getelementptr inbounds [2 x i32], [2 x i32]* %18, i32 0, i64 1

; LDC 1.31
%14 = getelementptr inbounds [2 x i32], [2 x i32]* %13, i64 0
%15 = bitcast [2 x i32]* %14 to i32*

@thewilsonator
Copy link
Contributor

Does this issue still happen when opaque pointers are enabled?

@kinke
Copy link
Member

kinke commented Apr 3, 2023

We're hitting

ldc/gen/binops.cpp

Lines 27 to 28 in 0d4d711

assert((offset % elemSize) == 0 &&
"Expected offset by an integer amount of elements");

The problem is that the pointer is int[2]*, and we're not expecting constant byte offsets (here: 4) NOT maching the element size. The frontend here 'flattens' the int[2]* to the resulting int*, where we'd correctly apply the GEP offset of 1.

@JohanEngelen
Copy link
Member

Yeah, the AST is invalid, but that's hard to fix now?

kinke added a commit to kinke/ldc that referenced this issue Apr 4, 2023
kinke added a commit to kinke/ldc that referenced this issue Apr 4, 2023
kinke added a commit to kinke/ldc that referenced this issue Apr 4, 2023
kinke added a commit to kinke/ldc that referenced this issue Apr 16, 2023
kinke added a commit that referenced this issue Apr 16, 2023
kinke added a commit to symmetryinvestments/ldc that referenced this issue May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants