[PATCH] D148042: [MSP430] Get the DWARF pointer size from MCAsmInfo instead of DataLayout.

[Ilia Kuklin via Phabricator via llvm-commits]

kuilpd added a comment.

In D148042#4287909 <https://reviews.llvm.org/D148042#4287909>, @dblaikie wrote:

>> If the patch just fixes what was overlooked, then changing directly is the way to go.
>
> The hope/desire would be to make it harder to overlook, by not having the DWARF code reach out to these various config places & just have to "know" which one was the right one - but have it handled once in DwarfDebug and then only consult that everywhere else.
>
> Could you look up the original commits that overlooked this and include references to them here? It'd help provide context for this change for both the review and commit/history later on. (explain how we got here, demonstrate these were overlooked/the changes proposed are consistent with that previous work, etc)



In D148042#4287909 <https://reviews.llvm.org/D148042#4287909>, @dblaikie wrote:

>> If the patch just fixes what was overlooked, then changing directly is the way to go.
>
> The hope/desire would be to make it harder to overlook, by not having the DWARF code reach out to these various config places & just have to "know" which one was the right one - but have it handled once in DwarfDebug and then only consult that everywhere else.
>
> Could you look up the original commits that overlooked this and include references to them here? It'd help provide context for this change for both the review and commit/history later on. (explain how we got here, demonstrate these were overlooked/the changes proposed are consistent with that previous work, etc)

As far as I could find, these changes were initially suggested in D11969 <https://reviews.llvm.org/D11969> (for different reasons, though) but were abandoned, then reintroduced in D30879 <https://reviews.llvm.org/D30879>.
However, not everything was reintroduced, like for example line 137 in AsmPrinterDwarf.cpp from D11969 <https://reviews.llvm.org/D11969>.

Regarding single entry point, I made it look a bit better (just like in D30879 <https://reviews.llvm.org/D30879>). On top of that I could make a getCodePointerSize() in AsmPrinter.h which would take the pointer size from MCAsmInfo, right next to getPointerSize() function which takes the pointer size from DataLayout. But at this point there is already an entry point, it's just in MCAsmInfo.


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

https://reviews.llvm.org/D148042