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

CBE can't handle programs with llvm.memcpy.i64 and llvm.memcpy.i32 #1510

Closed
lattner opened this issue Jan 28, 2007 · 5 comments
Closed

CBE can't handle programs with llvm.memcpy.i64 and llvm.memcpy.i32 #1510

lattner opened this issue Jan 28, 2007 · 5 comments
Labels
bugzilla Issues migrated from bugzilla compile-fail Use [accepts-invalid] and [rejects-valid] instead

Comments

@lattner
Copy link
Collaborator

lattner commented Jan 28, 2007

Bugzilla Link 1138
Resolution FIXED
Resolved on Feb 22, 2010 12:43
Version trunk
OS All
CC @nlewycky

Extended Description

The CBE is emitting:

unsigned char *memcpy(unsigned char *, unsigned char *, unsigned int );
unsigned char *memcpy(unsigned char *, unsigned char *, unsigned long long );

which causes the C compiler to barf.

This prevents burg from working with the CBE and also prevents bugpoint from working, which doesn't
allow us to track down why jit/llc also fail.

-Chris

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 29, 2007

This patch fixes the problem:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070122/043420.html

I'm not completely happy with it, however. It just forces memcpy and memmove to
be 32-bit. This might not work so well with large "count" values on a 64-bit
machine as the high 32-bits are being truncated. However, since the only clients
are CBE and LLI interpreter, and both target 32-bit machines, this shouldn't
cause any significant problems.

The real question is .. why does llvm generate both llvm.memcpy.i32 and
llvm.memcpy.i64 in the same program?

@lattner
Copy link
Collaborator Author

lattner commented Jan 29, 2007

The patch completely breaks 64-bit targets, please revert it. It should use targetdata to figure out the
size of a pointer, and use that as the size_t value.

-Chris

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 29, 2007

So, how do you unbreak llvm.memcpy.64 on a 32-bit platform? Truncating the value
down to 32-bits is also wrong.

@lattner
Copy link
Collaborator Author

lattner commented Jan 29, 2007

No it isn't. Clearly the value has to be dynamically 32-bits or smaller if your pointer is only 32-bits.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 30, 2007

A proper fix for this was made with these patches:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070129/043434.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070129/043435.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070129/043436.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070129/043437.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070129/043438.html

This endows IntrinsicLowering with TargetData and uses the IntPtrTy to get the
right prototype for memcpy/memmove/memset given the intended target. It also
adjusts CBE and LLI to pass the TargetData to IntrinsicLowering.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
troelsbjerre pushed a commit to troelsbjerre/llvm-project that referenced this issue Jan 10, 2024
[lldb] Fix compile error due implicit StringRef -> std::string conversion
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla compile-fail Use [accepts-invalid] and [rejects-valid] instead
Projects
None yet
Development

No branches or pull requests

2 participants