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

Deoptimize TLS access #17800

Merged
merged 1 commit into from
Aug 5, 2016
Merged

Deoptimize TLS access #17800

merged 1 commit into from
Aug 5, 2016

Conversation

yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Aug 4, 2016

* Apparently `ifunc` is not supported by gcc 5 on ARM
* Try even harder to workaround LLVM bug when dealing with returntwice function
// It's also harder to emit the offset in a generic way on AArch64
// (need to generate one or two `add` with shift) so let llvm emit
// the add for now.
auto T_size = (sizeof(size_t) == 8 ? Type::getInt64Ty(ctx) :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there aarch64 variants without 8 byte pointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is moved here from the above and I'm keeping it as close as possible to what it used to be with the hope that the LLVM bug can be fixed and we can revert all these workarounds.

@timholy
Copy link
Member

timholy commented Aug 4, 2016

Can we merge this? julia seems very unstable right now.

@yuyichao
Copy link
Contributor Author

yuyichao commented Aug 4, 2016

I got the clobber list wrong the first time so I'd like someone else familiar with x86 asm to double check. Should be good to go otherwise (especially since we've branched).

// The add instruction clobbers flags
auto tp = InlineAsm::get(FunctionType::get(T_pint8, false),
asm_str.c_str(),
"=r,~{dirflag},~{fpsr},~{flags}", false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this clobber the fpu status register? also, afaict, dirflags is deprecated and a no-op

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly because this is what clang generates and I couldn't find a single documentation of what each one mean and can't really figure out the logic in the x86 target code that uses these flags..

@vtjnash
Copy link
Member

vtjnash commented Aug 5, 2016

lgtm

@yuyichao yuyichao merged commit 2d30203 into master Aug 5, 2016
@yuyichao yuyichao deleted the yyc/codegen/deopt-tls branch August 5, 2016 21:49
tkelman pushed a commit that referenced this pull request Aug 11, 2016
* Apparently `ifunc` is not supported by gcc 5 on ARM
* Try even harder to workaround LLVM bug when dealing with returntwice function

(cherry picked from commit f90eab3)
ref #17800
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 this pull request may close these issues.

4 participants