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

Instcombine drops range information when combining inttoptr with load #46587

Closed
qcolombet opened this issue Aug 19, 2020 · 4 comments
Closed
Labels
bugzilla Issues migrated from bugzilla confirmed Verified by a second party llvm:instcombine

Comments

@qcolombet
Copy link
Collaborator

Bugzilla Link 47243
Version trunk
OS All
Attachments Reproducer
CC @fhahn,@jdoerfert

Extended Description

When instcombine replaces inttoptr(load intTy) to (load ptrTy) it drops the range information attached to the load.
More precisely, it replaces the range information with an inferior metadata.

The problem may lay into what the range information can represent, but we should fix that one way or another as this may result in suboptimal code.

  • Example *

Consider the following snippet:

define float* @​test5_range(i64* %ptr) {
entry:
  %val = load i64, i64* %ptr, !range !{i64 64, i64 65536}
  %valptr = inttoptr i64 %val to float*
  ret float* %valptr
}

The returned pointer is known to be between address 64 and address 65536.

After instcombine however, this information is lost and instead we have a less rich information that the pointer is non-null:

define float* @​test5_range(i64* %ptr) {
entry:
  %0 = bitcast i64* %ptr to float**
  %val1 = load float*, float** %0, align 4, !nonnull !​0
  ret float* %val1
}
  • To Reproduce *

opt -S -instcombine ~/Downloads/instcombine_range.ll -o -

@jdoerfert
Copy link
Member

This requires a lang ref change for !range or ptr2int and llvm.assume.
Either seems reasonable on first glance.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@msebor
Copy link
Contributor

msebor commented Sep 1, 2022

The expected range seems to be preserved on trunk (16.0) so it this must have been fixed. It might be worth adding the test to the test suite if one doesn't cover this case yet.

$ (set -x && cat llvm-pr46587.ll && opt -S -instcombine llvm-pr46587.ll)
+ cat llvm-pr46587.ll
define float* @f(i64* %ptr) {
entry:
  %val = load i64, i64* %ptr, !range !{i64 64, i64 65536}
  %valptr = inttoptr i64 %val to float*
  ret float* %valptr
}
+ opt -S -instcombine llvm-pr46587.ll
; ModuleID = 'llvm-pr46587.ll'
source_filename = "llvm-pr46587.ll"

define float* @f(i64* %ptr) {
entry:
  %val = load i64, i64* %ptr, align 4, !range !0
  %valptr = inttoptr i64 %val to float*
  ret float* %valptr
}

!0 = !{i64 64, i64 65536}

@qcolombet
Copy link
Collaborator Author

Hi @msebor,
From what you shared it looks like the reason the range is correct is because no combine has been applied.
Maybe that was an intentional fix to preserve the range information or the reproducer is not enough to expose the issue anymore.

@msebor
Copy link
Contributor

msebor commented Sep 13, 2022

Those seem plausible. I don't have a way to track down the change that preserves the range. In traiging these old reports I've made the assumption that it's helpful to close those that are not reproducible. Please let me know if that's not so or feel free to reopen this report if you think it might still serve a purpose.

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 llvm:instcombine
Projects
None yet
Development

No branches or pull requests

5 participants