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

Test back::x86_64::get_target_strs on a 64-bit Windows machine #2398

Closed
catamorphism opened this Issue May 17, 2012 · 7 comments

Comments

Projects
None yet
7 participants
@catamorphism
Contributor

catamorphism commented May 17, 2012

The data_layout field in the record that get_target_strs returns has a FIXME that says "Test this, copied from Linux".

@ghost ghost assigned graydon May 17, 2012

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix May 2, 2013

Member

I do not know much about LLVM, so I was mystified by these strings when I first looked at them.

(Looking at the LLVM source did not help, as the parameter names might make one think that these data layouts are just strings representing triples. Not true.)

But here is the LLVM docs on the format, which I found enlightening: http://llvm.org/docs/LangRef.html#data-layout

(Of course, this does not actually tell us whether we are selecting the right values on Windows, which is the whole point of this bug.)

Member

pnkfelix commented May 2, 2013

I do not know much about LLVM, so I was mystified by these strings when I first looked at them.

(Looking at the LLVM source did not help, as the parameter names might make one think that these data layouts are just strings representing triples. Not true.)

But here is the LLVM docs on the format, which I found enlightening: http://llvm.org/docs/LangRef.html#data-layout

(Of course, this does not actually tell us whether we are selecting the right values on Windows, which is the whole point of this bug.)

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix May 2, 2013

Member

(Also, we do already use llvm::LLVMSetTarget(llmod, buf) to pass in the target triple, which I infer could be sufficient... do we want/need to be maintaining these data layout strings? On all targets? Or should we switch to a model were we only call setDataLayout in specific cases where we determine it is necessary?)

Member

pnkfelix commented May 2, 2013

(Also, we do already use llvm::LLVMSetTarget(llmod, buf) to pass in the target triple, which I infer could be sufficient... do we want/need to be maintaining these data layout strings? On all targets? Or should we switch to a model were we only call setDataLayout in specific cases where we determine it is necessary?)

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix May 2, 2013

Member

(Then again, comments like this make me think that its better to let sleeping dogs lie, in case LLVM does happen to actually use this for optimizations we rely on. I guess I need to do more digging, or trust that the code works and focus just on the goal at hand: Testing windows.)

Member

pnkfelix commented May 2, 2013

(Then again, comments like this make me think that its better to let sleeping dogs lie, in case LLVM does happen to actually use this for optimizations we rely on. I guess I need to do more digging, or trust that the code works and focus just on the goal at hand: Testing windows.)

@cmr

This comment has been minimized.

Show comment
Hide comment
@cmr

cmr Jul 12, 2013

Member

Triage bump

Member

cmr commented Jul 12, 2013

Triage bump

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack Sep 25, 2013

Contributor

triage bump. nothing to add.

Contributor

metajack commented Sep 25, 2013

triage bump. nothing to add.

@dguenther

This comment has been minimized.

Show comment
Hide comment
@dguenther

dguenther Jan 18, 2014

Contributor

Does this still need a FIXME? I'm not an LLVM expert, but I did a bit of searching and found that the datalayout is the same as what's used in LLVM tests for target triple x86_64-pc-win32 ([1] [2] [3]).

Contributor

dguenther commented Jan 18, 2014

Does this still need a FIXME? I'm not an LLVM expert, but I did a bit of searching and found that the datalayout is the same as what's used in LLVM tests for target triple x86_64-pc-win32 ([1] [2] [3]).

@thestinger

This comment has been minimized.

Show comment
Hide comment
@thestinger

thestinger Sep 24, 2014

Contributor

This works fine and is being tested.

Contributor

thestinger commented Sep 24, 2014

This works fine and is being tested.

@thestinger thestinger closed this Sep 24, 2014

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