- User Since
- Jan 2 2013, 4:34 PM (403 w, 3 d)
Fri, Sep 25
I'd prefer it if we didn't have to guess the path style in the first place. Initially I thought the target triple might be able to help us out, but it doesn't, because really you want to know the path style of the file system in use where compilation took place. Normally, clang is the one reading the source files, so whatever clang's host's style is is the right style. These test cases are getting tripped up because compilation is split over two host OSs: Clang's OS and llc's OS. Now I wonder if we shouldn't just persist the path style through the IR module, or even through the assembler if DWARF needs to join paths after assembly.
Thu, Sep 24
I actually forgot about this change entirely and uploaded and landed a new one:
llvm::call_once is fine, but IMO the static local version is preferable: it's built in to the language, no headers to include.
Wed, Sep 23
In any case, if you want to do that separately, I think this change stands on its own, so I will approve it as is.
Looks good to me, I didn't review very in depth, but I see the test case that we need. :)
I'm saying the fix belongs in TargetObjectFileLoweringImpl.cpp, which is the point where we translate from IR to assembly. We can't fix this after producing assembly, because we don't know what symbols are marked dllimport at that point.
I think this is reasonable. There is no need to record call graph profile edges from functions to imported functions in the metadata. However, I would prefer to move the check into CodeGen, so that if we receive some IR that has edges like this, we don't emit an object file that cannot be linked. I'd be OK doing the check in both places.
I think we can go forward with the reviewers we have. I have one more concern. Are the other reviewers happy?
Arthur, do you mind taking a look?
Tue, Sep 22
Seems reasonable, if the new multi-value phi works with later passes.
I can take a look.
Looks good to me. My only question is whether to optimistically assume these work out of the box on Mac, and to relax the asserts. Let's wait for input from Nico, though.
Mon, Sep 21
Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them.
Fri, Sep 18
I see the assembler support is already there, looks like it works.
Thu, Sep 17
- rebase over previous changes
- fix funcIdToType map for /Yu files
- Ensure all objs with symbols have TpiSources
Wed, Sep 16
Thanks, looks like I can remove it. I made things more uniform by having ipiMap always point to something, so if we have an item reference, always use ipiMap.
- remove debugging noinline
- eliminate CVIndexMap entirely
Here is the sequence of what happened as I understand it:
- D71687 is intended to fix the full loop unrolling pragma with the NPM at -O1. The accompanying clang test covers this.
- D71687 also adds an LLVM IR test. It is an integration test: it started from unoptimized IR and ran the full -O1 NPM pipeline. The IR happens to contain optnone. Maybe that was unintentional, since one used to be able to run clang foo.c -emit-llvm -o - | opt -O1 -S and get optimized IR. Now clang applies the optnone attribute, and that no longer works.
- D85578 made the IR test more targeted: now it only runs the full unrolling pass instead of the O1 pipeline.
Yeah, my goal is for the build system to be able to avoid having to embed PWD into the compiler flags. For example, I believe it is a goal of the LLVM gn build system for the build tree to be relocatable. If the compiler command lines need to contain absolute paths to make the paths in the coverage relative, we won't be able to achieve that goal.
Tue, Sep 15
The situation where a temp file won't be delete without the delete-on-close bit set seems rare so just avoid setting it entirely.
Fri, Sep 11
Looks good, but we should test both sides of the MSVC behavior.
Thu, Sep 10
I would ask you to add a test, but I can't recommend the existing test, llvm/test/DebugInfo/COFF/types-array.ll, as a good basis for a test. I'll look into rewriting it and adding coverage after you land.
Wed, Sep 9
Jun 30 2020