- User Since
- Sep 26 2016, 7:58 AM (99 w, 10 h)
Fri, Aug 17
Thu, Aug 16
I've got another solution for this problem for our out-of-tree-target. So I'll abandon this.
I was going to use this in another patch, but it's outdated now so I'll abandon this.
Just adding some of the latest commiters to RegisterCoalescer.cpp as reviewers. I hope someone is able to review this.
Wed, Aug 15
Mon, Aug 13
Fri, Aug 10
Thu, Aug 9
Sun, Aug 5
I think that it is about time to at least remove MCRegisterClass::getSize().
Notice that I also suggested to remove getPhysRegSize() (and the PhysRegSize member) in https://reviews.llvm.org/D47199, but it is perhaps more controversial to remove the register size completely from MCRegisterClass?
Afaik there is not in-tree use of getPhysRegSize(). And making it possible to get the size in bytes of a reg class with non-byte-sized registers, without telling that it truncates down to whole bytes, might be a little bit confusing (e.g. getPhysRegSize() for a register class with 1-bit registers will return 0).
Tue, Jul 31
Jul 20 2018
Jul 11 2018
This seems to work quite nice for me now! Nice!
Jul 10 2018
Jul 9 2018
Jul 6 2018
Jul 5 2018
FYI: Looks like you have address my earlier comments. So I have no blocking remarks.
Jul 4 2018
Jul 3 2018
Can't build unittests (I suspect this patch):
Jul 2 2018
I tried running
I don't know much about the BlockAddress concept. The LangRef says things like "always has an i8* type" and "this may be passed around as an opaque pointer sized value". But I guess it would be weird if the size doesn't match the size of pointers in the program address space, so the patch makes sense to me.
Jun 28 2018
Jun 27 2018
Jun 26 2018
Jun 25 2018
Update after review comments:
- Use Optional in AllocaInst::getAllocationSizeInBits.
- Add comments describing the added test cases.
Jun 21 2018
We all have some suspicions that it might be awkward to move a DBG_VALUE intrinsic across another one for the *same* *variable*. But we haven't seen that happen in this particular situation when having an end-to-end test with C/C++ input. I'm pretty sure such things can happen in other passes (e.g. CodegenPrepare is moving dbg.value without any checks at all), so it is more like a general concern.
Jun 20 2018
AFAIK BasicAA assumes that all GEP indices have a common type. So "normalization" is for example needed before passes like GVN that uses MemoryDependenceResults, that is using BasicAA. Or maybe that is a bug in BasicAA?
Jun 19 2018
Jun 18 2018
Jun 15 2018
Jun 14 2018
Jun 13 2018
After some more testing I discovered a case when the added assert in
void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII, PHINode *APN, DIBuilder &Builder)
Jun 12 2018
Basically looks good to me now, as I think that getting rid of the debug-invariant problem is important.
When it comes to the sinking of DBG_VALUE instructions I believe that will be as good as the pre-RA version of MachineSinking. I think that could be enough to land this (as getting rid of the debug-invariance probably is more important).
More updates after review.
Jun 11 2018
Updated after comments from aprantl.
This comment in the description does not make sense to me: