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

Variable not visible at Og #50901

Closed
FiorellaArtuso opened this issue Aug 20, 2021 · 7 comments
Closed

Variable not visible at Og #50901

FiorellaArtuso opened this issue Aug 20, 2021 · 7 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla confirmed Verified by a second party debuginfo llvm:transforms

Comments

@FiorellaArtuso
Copy link

Bugzilla Link 51559
Version trunk
OS Linux
CC @dwblaikie,@JDevlieghere,@jmorse,@walkerkd,@pogo59

Extended Description

Comment:
When stepping on line 4 the variable g_3012 is not visible.

Steps to reproduce bug:

root@e60e0a2bba1a:/home/stepping/output# clang -v
clang version 14.0.0 (https://github.com/llvm/llvm-project.git 9573343)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7.5.0
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/8
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7.5.0
Candidate multilib: .;@m64
Selected multilib: .;@m64

root@e60e0a2bba1a:/home/stepping/output# lldb -v
lldb version 14.0.0 (https://github.com/llvm/llvm-project.git revision 9573343)
clang revision 9573343
llvm revision 9573343

root@e60e0a2bba1a:/home/stepping/output# cat a.c
int g_97 ;
static short g_3012 ;
void func_30() {
g_97 = g_3012;
}
int main() {
{
func_30();
}
}

root@e60e0a2bba1a:/home/stepping/output# cat -n a.c
1 int g_97 ;
2 static short g_3012 ;
3 void func_30() {
4 g_97 = g_3012;
5 }
6 int main() {
7 {
8 func_30();
9 }
10 }

root@e60e0a2bba1a:/home/stepping/output# clang -g -Og a.c -o opt

root@e60e0a2bba1a:/home/stepping/output# lldb opt
(lldb) target create "opt"
Current executable set to '/home/stepping/output/opt' (x86_64).
(lldb) b main
Breakpoint 1: where = opt`main [inlined] func_30 at a.c:4:15, address = 0x0000000000400490
(lldb) r
Process 68731 launched: '/home/stepping/output/opt' (x86_64)
Process 68731 stopped

  • thread #​1, name = 'opt', stop reason = breakpoint 1.1
    frame #​0: 0x0000000000400490 opt`main [inlined] func_30 at a.c:4:15
    1 int g_97 ;
    2 static short g_3012 ;
    3 void func_30() {
    -> 4 g_97 = g_3012;
    5 }
    6 int main() {
    7 {
    (lldb) p g_3012
    error: expression failed to parse:
    error: <user expression 0>:1:1: use of undeclared identifier 'g_3012'
    g_3012
@dwblaikie
Copy link
Collaborator

Looks like IPSCCPPass removes the global variable & doesn't create a constant expression for the debug representation (DIGlobalVariableExpression) representing the value.

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 24, 2021

Yes, this is a bug in ISCCP as Dave said.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@CarlosAlbertoEnciso CarlosAlbertoEnciso self-assigned this Sep 18, 2023
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Oct 13, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Oct 13, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Address reviewers comments:
- Refactor the new code into a helper function.
- Use early returns to reduce nested ifs.
- Remove C-style casts.
- Add validation to detect posible nullopt.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Oct 13, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Address reviewers comments:
- Refactor the new code into a helper function.
- Use early returns to reduce nested ifs.
- Remove C-style casts.
- Add validation to detect posible nullopt.
- Collapse conditions in a single if.
- Remove 'has_value'.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Oct 13, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Address reviewers comments:
- Refactor the new code into a helper function.
- Use early returns to reduce nested ifs.
- Remove C-style casts.
- Add validation to detect posible nullopt.
- Collapse conditions in a single if.
- Remove 'has_value'.
- Additional if condition simplifications.
- Move new logic into a general helper function.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Oct 13, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Address reviewers comments:
- Refactor the new code into a helper function.
- Use early returns to reduce nested ifs.
- Remove C-style casts.
- Add validation to detect posible nullopt.
- Collapse conditions in a single if.
- Remove 'has_value'.
- Additional if condition simplifications.
- Move new logic into a general helper function.
- Move new logic to the 'Dbg Intrinsic utilities'" section.
- Pass 'Constant' and 'Type' as references.
- Remove extra whitespace differences (incorrect clang-format).
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Oct 13, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Address reviewers comments:
- Refactor the new code into a helper function.
- Use early returns to reduce nested ifs.
- Remove C-style casts.
- Add validation to detect posible nullopt.
- Collapse conditions in a single if.
- Remove 'has_value'.
- Additional if condition simplifications.
- Move new logic into a general helper function.
- Move new logic to the 'Dbg Intrinsic utilities'" section.
- Pass 'Constant' and 'Type' as references.
- Remove extra whitespace differences (incorrect clang-format).
- Use 'Instruction::IntToPtr' with pointer types.
- Reduce lit test case.
- Add unittests for the new helper function.
CarlosAlbertoEnciso added a commit that referenced this issue Oct 24, 2023
https://bugs.llvm.org/show_bug.cgi?id=51559
#50901

IPSCCP pass removes the global variable and does not create a constant
expression for the initializer value.
@hashemthomas1
Copy link

I recently encountered a crash which was a result of the patch that closed this issue.

The crash happens in the function getExpressionForConstant that SCCP pass utilizes, when reaching the following code section that deals with floating point values.

if (Ty.isFloatTy() || Ty.isDoubleTy()) {
    const APFloat &APF = cast<ConstantFP>(&C)->getValueAPF();
    return DIB.createConstantValueExpression(
        APF.bitcastToAPInt().getZExtValue());
  }

It specifically happens in the cast operation when the value of the float is undef.

A very simple OpenCL reproducer of the crash with x86 architecture: https://godbolt.org/z/sTqaW6MvE

@Endilll
Copy link
Contributor

Endilll commented Dec 29, 2023

I mentioned the comment above in #66745

@fhahn
Copy link
Contributor

fhahn commented Dec 31, 2023

Fixed the crash in b46638d. @CarlosAlbertoEnciso please could you check if there's more appropriate debug data for undef/poison. Also, the original code only checked for float and double FP types, but there are others, like half. Could you extend the test coverage to include those types as well?

@CarlosAlbertoEnciso
Copy link
Member

@fhahn Thanks for fixing the crash. I just returned this week from PTO.

CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Jan 12, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Extend test coverage to include:
- half and bfloat types.
- checks for undef (int32 and ptr).
@CarlosAlbertoEnciso
Copy link
Member

Uploaded a patch to extend the test coverage for half, bfloat and undef.
#77901

CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Jan 19, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Extend test coverage to include:
- half and bfloat types.
- checks for undef (int32 and ptr).
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Jan 19, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

- Extend test coverage for all float types:
  Half, bfloat, fp128, x86_fp80, ppc_fp128
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Jan 19, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Fix assertion detected by the BuildKite pre-commit CI:

llvm::APInt::getZExtValue() const:
`getActiveBits() <= 64 && "Too many bits for uint64_t"' failed.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Apr 9, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Extend test coverage to include:
- half and bfloat types.
- checks for undef (int32 and ptr).
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Apr 9, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

- Extend test coverage for all float types:
  Half, bfloat, fp128, x86_fp80, ppc_fp128
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Apr 9, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Fix assertion detected by the BuildKite pre-commit CI:

llvm::APInt::getZExtValue() const:
`getActiveBits() <= 64 && "Too many bits for uint64_t"' failed.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Apr 9, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Drop support for fp128, ppc_fp128 and x86_fp80 types.
CarlosAlbertoEnciso added a commit to CarlosAlbertoEnciso/llvm-project that referenced this issue Apr 16, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
llvm#50901

IPSCCP pass removes the global variable and does not
create a constant expression for the initializer value.

Address @SLTozer comments:
- Reinstate the test (FP >= 64) for nullptr expressions.
- Change condition to clarify the FP size restrictions.
- Use 'getZExtValue' instead of 'tryZExtValue'.
CarlosAlbertoEnciso added a commit that referenced this issue Apr 16, 2024
https://bugs.llvm.org/show_bug.cgi?id=51559
#50901

IPSCCP pass removes the global variable and does not create a constant
expression for the initializer value.

Extend test coverage to include:
- half, bfloat types.
- checks for undef (int32 and ptr).

There is no support for:
- fp128, x86_fp80, ppc_fp128  types.
#88102
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla confirmed Verified by a second party debuginfo llvm:transforms
Projects
None yet
Development

No branches or pull requests

8 participants