- User Since
- Mar 2 2013, 8:12 AM (307 w, 1 d)
Fri, Jan 18
Thu, Jan 17
Wed, Jan 16
I think we can reduce the test even more, but otherwise this should be fine.
Seems reasonable to me.
Thanks for the patch! The testcase looks like it would bitrot very easily. I would recommend first manually reducing the IR as much as possible and then making a MIR testcase out of it to become independent from future ISEL changes.
Looks like this test is failing on green dragon:
Tue, Jan 15
I changed all the autos back to full types in r351237.
Thanks Pavel, your argument of optimizing storage over interfaces makes perfect sense. Using an Optional instead.
I didn't realize you were talking about DwarfExpression, sorry . I'm not really sure what the best solution is because I don't quite understand MCLabels that well. Naively I was assuming that we would emit an MCLabel that is defined as the difference between DIE offset and .debug_info start label. Since we need to emit DIE references all over .debug_info I would start by looking at how it's done there. The fact that it is in a different section may complicate things though, I'm not sure.
Roughly: the implementation of print used code (MachineInstr operator<<) that lives in the CodeGen library, and various tools that don't link against CodeGen include header files from the CodeGen Clang module). Without local submodule visibility, this pulls in all headers of the CodeGen Clang module, thus causing code to be generated for the dump function, which then causes the linker error.
Mon, Jan 14
Seems okay to me, but it would be good to wait Björn's LGTM.
I *think* I fixed it for good in in r351077.
Fri, Jan 11
ii) It is probably not safe to assume that the consumer/debugger would set the high bits to zero in zero extension in e.g. the case that the variable has been spilled to memory.
Thu, Jan 10
Wed, Jan 9
You have to use clang in order to compile with clang module support enabled. If your host compiler is GCC, you will have to build clang first and then use that clang to compile clang again using all the MODULE cmake options.
Tue, Jan 8
It should not be mac-specific, but it is possible that Linux uses a different default for LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY. Can you try again with -DLLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY=0 -DLLVM_ENABLE_MODULES=1 ?
Hmmm.. my fix didn't go far enough. I reverted this patch once again, so we can sort this out without pressure. As long as you make sure that compiling with -DLLVM_ENABLE_MODULES=1 works, feel free to recommit at any time!
I committed a rather horrible workaround in r350650, but it would be great to revert that again.
Mon, Jan 7
There's a type in a test, but otherwise, this LGTM.
Looks like these are unrelated failures. TestQueues started acting up last week because of a concurrency issue. I would recommend you attempt to re-land your commit again and see if any new failures show up. If you are unsure, feel free to ask here or on #llvm on IRC.
Fri, Jan 4
Thu, Jan 3
Is the problem that it is a SubroutineType, but not a function *pointer*?
Generally, if the Verifier accepts it we ought to not crash, so this seems to be okay, but it might be worth asking whether this is really what you want your frontend to be creating since subroutinetypes don't have a size, etc.
A testcase that triggers this situation would be nice, too.
I implemented a poor man's version of that in r350360.
Wed, Jan 2
Dec 21 2018
Dec 20 2018
Looks generally good to me.
Dec 19 2018
Dec 18 2018
Sorry I forgot to comment: this should be fixed in r349533.
Ah.. that's possible. REQUIRES needs to be very high up in the file, but I don't know what the exact threshold is
I also found that the tools (e.g. LLVM or LLDB) are eager to know about number of registers (which we cannot determine) to allocate internal structures.
Can you confirm that reverting this path actually fixes the issue? I'm asking because the only test that is executing this script has a REQUIRES: system-darwin line in it.
Dec 17 2018
Got rid of the no_uuid regex and added even more testcases.
Dec 15 2018
Fixed the reported problems with the regexes and added testcases with example lines. I used the funny test setup to avoid burdening the installed crashlog.py script with unit test code that would need to be parsed every time.
Dec 14 2018
Dec 13 2018
Looks like this caused a noticeable shrinking of location lists, but that means we're getting more accurate: http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607042.4&highlight_run=117379 !
Can you watch http://lnt.llvm.org/db_default/v4/nts/117379 (http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607042.4 and http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607043.4) after landing this?
Please be sure to watch all LLDB bots (green dragon and lab.llvm.org:8011) closely when landing this change.
Let's give this a try.
I believe I fixed this in r349065. (The DIFile used by the CU is special and distinct from the main source file. Its directory part specifies what becomes the DW_AT_comp_dir (the compilation directory), even if the source file was specified with an absolute path.)
Dec 12 2018
Be more verbose about what is a valid arch.
Dec 11 2018
This one is probably less controversial than D55574 :-)
I'm mostly on board with making these changes, as it's good LLVM style to do this. I highlighted a couple changes that might warrant a closer look.
I just verified that this is no longer applicable as there is no longer a DW_OP_deref in the inlined function.
Looks like this landed in r345274 and wasn't properly closed.
I also removed the other references to DIBuilder::createFile() in r348866.
Dec 10 2018
LGTM with outstanding comment addressed.
Dec 7 2018
Thanks, should be fixed in r348610!
LGTM if @vsk's question is answered satisfactorily.