User Details
- User Since
- Jan 2 2013, 4:34 PM (495 w, 5 d)
Thu, Jun 30
Should these be unnamed labels so they don't appear in the COFF file symbol table? I'm guessing ml64 doesn't produce symbols for these labels, but correct me if I'm wrong.
Thu, Jun 23
Test cases look good, thanks!
Wed, Jun 22
Thanks for expanding our docs!
Fri, Jun 17
+1 for the exception code. I think it may appear as the process return code and some systems will log that, but logging it ourselves, too, is for the best.
Thu, Jun 16
lgtm
Thanks, sorry for the delay.
I think Richard had some concerns in the other review that this may not be enough to really guarantee initialization order within the TU. I couldn't say either way, I shouldn't review this code. Conceptually, this change seems small enough to me. Can we ask @aaron.ballman to take a look?
Wed, Jun 15
Seems useful
I don't have a great answer here, but yes, dllexport macro norms sort of run directly counter to the normal ways that people use PCH. It seems like projects will need to have a library module / PCH file for building a DLL and then a PCH / module for consuming the DLL.
Thanks, this is a good idea.
Tue, Jun 14
Mon, Jun 13
lgtm
Fri, Jun 10
Does the MLIR LLVM dialect not have comdats? That seems like a missing feature, a lack of fidelity that would prevent lossless conversion from LLVM IR to MLIR & back. Clang uses comdat groups for some esoteric features, the D5 destructor alias group, and MSVC RTTI.
lgtm
Thu, Jun 9
Tue, Jun 7
@labath, ping, I see you just got back from vacation and this is a lot of code, but any high level design feedback would be helpful to know if this is the right direction.
Jun 3 2022
lgtm, thanks
Jun 2 2022
lgtm
lgtm
Well, I guess we're out of luck, but that seems like a very poorly considered requirement from the standard. If we can't use comdats for inline variables, every time you include a header with a dynamically initialized variable, it will generate extra initialization code in every TU that cannot be optimized away. This reminds me of the problems people used to have where every TU including <iostream> emitted extra initialization code.
lgtm, sorry for the delay, do you need someone to push this?
May 31 2022
I think it's fine to implement this, but I wanted to share some of the motivation for the current state of things. In our experience, the majority of uses of pragma optimize were to work around MSVC compiler bugs. So, instead of honoring them, we felt it was best to ignore them with a warning (-Wignored-pragma-optimize). Of course, that warning is typically disabled in build systems of large projects, so our current behavior can still surprise users. Implementing the feature is probably the most predictable and least surprising thing Clang can do.
May 27 2022
lgtm
lgtm
May 26 2022
May 25 2022
lgtm
I'm somewhat supportive of the goal here, but I think there are still some underlying issues.
May 23 2022
I am reminded of the perennial problem of "optional" protobuf fields that, when omitted, will cause production crashes.
lgtm
lgtm
May 20 2022
@MaskRay, can you review this? I think you have been thinking the most about how to parallelize linkers, and what fundamental data structures are most important.
This seems reasonable to me, but surely this is not the complete list of places where we must use the VFS?
lgtm, thanks!
I agree with David, I would like to see LLVM move in the direction of never emitting empty functions. These are just labels that snap to the next function in the same section, and that's silly. I'm not sure what happens if you use function sections. We should just emit some trap instruction, and let the linker do identical code folding (ICF) to merge them back together. This will regress code size, but I doubt out users will complain, and ICF will recover most of the size regression.
I was unable to find any documentation for the meaning of AArch64 addrspace(256), and I wasn't able to figure it out after studying the code in llvm/lib/Target/AArch64 for ten minutes or so. The change seems fine, but please add some documentation as a follow-up. X86 has some of its address spaces documented here, not that this is the best place:
https://llvm.org/docs/CodeGenerator.html#x86-address-spaces-supported
May 18 2022
Nice!
May 16 2022
I haven't given any input yet because I haven't yet seen a reduced example of the conforming code that is broken by this change. If someone can put together a small godbolt example that shows the issue affecting ADL, I'd appreciate it.
Looks good to me. I generally agree with Adrian's suggestions.
May 13 2022
Sorry, I can't easily test this patch, but I do think it's probably the right direction.
May 12 2022
I wanted to give some early feedback that this broke our internal build of CPython, and we put together a local workaround, so we aren't blocked. I don't fully understand what's going on or have time to follow up, but I wanted to give a heads up that this seems like it could be a disruptive change for users.
May 11 2022
lgtm
lgtm
May 10 2022
This seems reasonable, but I worry about the consequences of creating lots of unnamed global variables. What will gdb do with so many unnamed globals? What will the PDB linker do with all these unnamed globals? I can't answer these questions, and you're welcome to try and find out, but I predict there will be problems, so just be aware of that possibility.
May 4 2022
May 3 2022
lgtm, thanks!
May 2 2022
Apr 29 2022
Which pass inserts the calls to these intrinsics? Can that pass be responsible for calculating funclet colors and adding the operand bundles to the intrinsics? That would match what we do for other instrumentation passes (ASan, PGO). The pre isel lowering pass can simply carry over the funclet operand bundle if present. I think there is a utility for carrying over that and similar operand bundles.
Apr 28 2022
I plugged this into godbolt to confirm the behavior of MSVC:
https://gcc.godbolt.org/z/v3P3WbxG3
Apr 27 2022
Looks good to me, just one style comment.
Apr 26 2022
Apr 25 2022
Apr 19 2022
lgtm
lgtm
Apr 18 2022
lgtm, thanks!
I read through the situation described in PR37807 the two relevant commits (rG9cc1ffadc5ad06ab846a7da95a1afb874b9f3d98 and D48444), and I think I understand the two issues.
This is sort of reasonable, but I feel like people might disagree so I won't go ahead and stamp it.
lgtm
lgtm
Apr 15 2022
It might be possible to recompute these, but I don't think it would be reliable.
Apr 14 2022
Thanks for running the experiments, I think I'm convinced we need to do this. @hans, can you review the implementation? I removed Adrian and Rui, who are no longer regular contributors.
Generally speaking, this sounds like a good idea to me. One time in 2019 I used -ftime-trace+ClangBuildAnalyzer and it told me that std::unique_ptr was the most expensive template because it is instantiated so much. Those results don't even capture the -O0 object file size impact.
Looks good to me after the fact.
How old is 19.20? Do we need a version compatibility check here to ensure that we don't break users of MSVC pre-19.20? I believe that is spelled LangOpts.isCompatibleWithMSVC(...).