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

fp stackifier broken #1384

Closed
nlewycky opened this issue Nov 19, 2006 · 7 comments
Closed

fp stackifier broken #1384

nlewycky opened this issue Nov 19, 2006 · 7 comments
Assignees
Labels
backend:X86 bugzilla Issues migrated from bugzilla miscompilation

Comments

@nlewycky
Copy link
Contributor

Bugzilla Link 1012
Resolution FIXED
Resolved on Feb 22, 2010 12:42
Version trunk
OS Linux
Attachments bugpoint reduced testcase, matching x86 assembly

Extended Description

Latest CVS (since about a week ago) broke a dozen tests in my nightly. Testcase
attached.

@nlewycky
Copy link
Contributor Author

assigned to @lattner

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 20, 2006

Nick,

I was getting something similar until I updated my CFE this morning. Did you
update your CFE and rebuild?

Reid.

@nlewycky
Copy link
Contributor Author

No, I rebuilt it right before building LLVM. I try to have a working LLVM before
building the CFE.

I'll try it now.

@nlewycky
Copy link
Contributor Author

No change.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 20, 2006

Builds and tests okay here. Dunno.

@lattner
Copy link
Collaborator

lattner commented Nov 20, 2006

Reduced testcase:

float %foo(float %col.2.0) {
%tmp = load float
%col.2.0 ; [#uses=3]
%tmp16 = setlt float %tmp, 0.000000e+00 ; [#uses=1]
%tmp20 = sub float -0.000000e+00, %tmp ; [#uses=1]
%iftmp.2.0 = select bool %tmp16, float %tmp20, float %tmp
ret float %iftmp.2.0
}

compiles to:

$ llvm-as < t.ll | llc -mcpu=i386
_foo:
movl 4(%esp), %eax
flds (%eax)
fld %st(0)
fchs
fldz
fucomip %st(0), %st(0)
fxch %st(1)
fcmovnbe %st(1), %st(0)
fstp %st(1)
#FP_REG_KILL
ret

Note that the fucomip compares two zeros against each other. This is probably fallout from Evan's
recent patch.

-Chris

@lattner
Copy link
Collaborator

lattner commented Nov 20, 2006

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
clementval pushed a commit to clementval/llvm-project that referenced this issue Jan 17, 2022
* [flang] Fix overallocation by fir-to-llvm-ir pass

When converting a fir.alloca of an array to the LLVM dialect, we used to
multiply the allocated size by all the constant factors encoded in the
array type. This is fine when the array type is converted to the element
type for the purposes of the allocation, but if it's converted to an
array type, then we might be allocating too much space. For example, for
`%2 = fir.alloca !fir.array<8x16x32xf32>, %0, %1` we would allocate
%0 * %1 * 8 * 16 * 32 x llvm.array<32 x array<16 * array<8 x f32>>>. We
really only need to allocate %0 * %1 such arrays.

This patch fixes the issue by taking note of the array type that we're
trying to allocate. It tries to match the behaviour of
LLVMTypeConverter::convertPointerLike, which returns a pointer to the
element type only when the array type doesn't have a constant interior.
We consequently only multiply with the constant factors in the array
type if the array type doesn't have a constant interior.

This has the nice side effect that it gets rid of some redundant
multiplications with the constant 1 in some cases.

Differential Revision: https://reviews.llvm.org/D116926

* Update Fir/alloc.fir

It just contained a redundant multiplication with 1.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla miscompilation
Projects
None yet
Development

No branches or pull requests

3 participants