[PATCH] D56297: [DWARFUnit] Don't assume basic types.
Andrew Kelley via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 4 15:41:15 PST 2019
andrewrk added inline comments.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:472
+ if (!BTy)
+ return false;
unsigned Encoding = BTy->getEncoding();
----------------
davide wrote:
> probinson wrote:
> > andrewrk wrote:
> > > davide wrote:
> > > > aprantl wrote:
> > > > > Why is false the right answer here?
> > > > To be quite honest, I think it's irrelevant (in the sense that it doesn't matter what we return here).
> > > It seems to me that the question "is a subroutine an unsigned type" is a nonsensical question, and should be an assertion error (as it is in status quo trunk).
> > >
> > > Or otherwise if a subroutine can be treated implicitly as a pointer, then it should have the same answer that a pointer would have, which is `true`.
> > I agree that asking whether a subroutine type is an unsigned type makes no sense. All contexts where we call isUnsignedDIType() are about emitting constant values. Why are we asking about a subroutine type?
> Is there anything that makes you think we should treat this implicitly as a pointer? I think this a very strong assumption to make in general.
If you're asking for my opinion on this issue, what I think should happen is:
* No change to DwarfUnit.cpp
* Verifier gains support to reject the example IR as invalid
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56297/new/
https://reviews.llvm.org/D56297
More information about the llvm-commits
mailing list