[PATCH] D73283: Handle complex DWARF expressions in combination with "complex" registers

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

I'd prefer if you didn't mix refactoring and functional changes (assuming that the changes to the DwarfExpression::Register struct could be done as an NFC regardless of what happens to this patch). Currently I think this is both a little bit hard to review, and it will become a mess to integrate downstream (even if that is hard for you to predict).

We got a couple of special cases that we currently need to handle downstream in addMachineRegExpression involving types that aren't a multiple of the byte size (we need to pad with extra pieces to satisfy the debugger). And for some reason we also need to reverse the order of all pieces since they should describe the variable in increasing memory order, and at least for our big-endian target (together with the iteration order of MCSubRegIterator) we get things in the wrong order unless we reverse the DwarfReg vector at the end of addMachineReg. Well, this is hopefully nothing that you need to care about, but right now it is hard for me to see how things would work when this patch has landed. And temporarily maintaining two implemenations would also be hard if the framework is refactored/renamed as part of the patch.

Sorry if I misunderstood something (if the DwarfExpression::Register changes wouldn't make sense at all as a standalone patch).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73283/new/

https://reviews.llvm.org/D73283