rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.05k stars 12.69k forks source link

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

Closed catamorphism closed 10 years ago

catamorphism commented 12 years ago

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

pnkfelix commented 11 years ago

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 commented 11 years ago

(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 commented 11 years ago

(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.)

emberian commented 11 years ago

Triage bump

metajack commented 11 years ago

triage bump. nothing to add.

dguenther commented 10 years ago

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 commented 10 years ago

This works fine and is being tested.