- User Since
- Jan 2 2013, 4:34 PM (255 w, 1 d)
Tue, Nov 21
Yep, looks good
Yep, looks good. :)
Mon, Nov 20
Didn't mean to push "accept"
Got it, just checking my understanding.
My hope was that we wouldn't need a flag. We'd just figure out if there were long section names that were marked to not be loaded at runtime (i.e. things that look like DWARF) and if so emit the long section table. Or, does DWARF need the full symbol table too?
Looks good, thanks for the heads up
This wasn't what I had in mind. When a frontend (or any other IR generator) creates a call, it should have a source of type information telling it the function prototype and the calling convention (in clang, the AST). It should also apply any other wacky calling convention flags like -mregparm and other things. In the special case where the frontend is emitting a call to a function defined in the current TU, we should have a check that the conventions match. I was imagining that you'd make IRBuilder implement that assert, and then add an alternative CreateCallWithConvention entry point or something like that to allow the frontend to override the check.
Fri, Nov 17
Do you think we should merge .idata, .edata, and .didata into .rdata as well? That will be more involved than this.
I'll go ahead and land this as requested to unblock things. Thanks for the patch, sorry for the trouble!
Ouch, thanks for the fix, looks good.
- adjust isEligble conditions
lgtm, let's give it a go.
Why does passing the rparen location for the if condition fix the warning for IF_ELSE? I assumed that would refer to the else clause semicolon.
Thu, Nov 16
Yep, no need for this.
Wed, Nov 15
This seems to cause a crash on startup in some gtest binaries when I self-host, so I guess I should debug that tomorrow before committing. The rest of clang's tests pass. I guess we don't use virtual inheritance. =S
No yaml support for import libraries, I take it?
I haven't dug into this code to really understand if this is right and won't change our version detection logic, but yes, broadly I believe we should just consult PATH, find a cl.exe, and check its version. I'd like to reduce this path validation to a minimum.
lgtm, definitely don't want to auto-export that imported data. The absolute symbol thing can be a follow-up change.
Tue, Nov 14
Do you mind digging a little bit so we can get a test case for it? Is there some way to get symbol definitions from .drectives like ELF with --defsym or something?
I think I'm in favor of ignoring options that reinforce LLD's defaults and don't have negatives. We can wait until a user files a bug and then add a negative option for them, or maybe add an escaping mechanism to let them pass the negative MSVC-style option.
Are all of these defaults important for mingw compatibility, or just -dynamicbase?
Looks good, sorry for the delay!
Thanks, looks good!
I rewrote this with salvageDebugInfo, take another look.
- rewrite with salvageDebugInfo
I wonder if we should limit the long section name extension to sections that aren't mapped at runtime. This makes sense because if some tool cares about unmapped section contents, they're going to need to look at the file on disk, not the image in memory, and that will have a full symbol table.
lgtm with suggestion
Mon, Nov 13
I checked, and we wrap this enum in our C API, so there should be no ABI concerns here. See LLVMValueKind LLVMGetValueKind(LLVMValueRef Val) in IR/Core.cpp.
Looks good. I think MachineInstr::isPosition is a more accurate way to identify these types of label instructions. If there is any code that relies on hasUnmodeledSideEffects, we can update it to check isPosition instead.
Sat, Nov 11
Fri, Nov 10
One last attempt at simplification
Rewrite one more time with fewer bools. The tricky case is /opt:noicf,ref.
- make opt:ref imply ICF unless it's explicitly disabled