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

Attributor special casing of return select interferes with nofpclass inference #63404

Closed
arsenm opened this issue Jun 20, 2023 · 1 comment
Closed

Comments

@arsenm
Copy link
Contributor

arsenm commented Jun 20, 2023

Noticed this when working on https://reviews.llvm.org/D153089

Attributor seems to specially treat return of select, such that computeKnownFPClass is never called on the select instruction. In this example, it fails to add nofpclass(nan) to the return value. If you obscure the return value with some kind of call, it works:

define float @ret_select_nnan_flag(i1 %cond, float %arg0, float %arg1) {
  %select = select nnan i1 %cond, float %arg0, float %arg1
  ret float %select
}

declare float @llvm.arithmetic.fence.f32(float)

define float @ret_fence_select_nnan_flag(i1 %cond, float %arg0, float %arg1) {
  %select = select nnan i1 %cond, float %arg0, float %arg1
  %fence = call float @llvm.arithmetic.fence.f32(float %select)
  ret float %fence
}

This will miss instruction flags and select clamping patterns computeKnownFPClass could recognize

jdoerfert added a commit to jdoerfert/llvm-project that referenced this issue Jun 30, 2023
The very first AA, at least the first one in order, is now gone.
AAReturnedValues was from a different time, one might say, a simpler
time. However, it was already rewamped once to use
`Attribute::getAssumedSimplifiedValues`, which is what the updated
AAPotentialValuesReturned now does too. To match the old behavior we
needed to avoid the helper AAReturnedFromReturnedValues and iterate the
return instructions explicitly, however, it is still less complexity
than it was before.

Fixes: llvm#63404
@jdoerfert
Copy link
Member

Proposed fix: https://reviews.llvm.org/D154917

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants