lebedev.ri added inline comments.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1008
+ // We ignore conversions to/from pointer and/or bool.
+ if (!(SrcType->isIntegerType() && DstType->isIntegerType()))
+ return;
----------------
erichkeane wrote:
> I'd rather !SrcType->isInt || !DestType->isInt
This i'd prefer to keep this as-is, since this is copied verbatim from
`ScalarExprEmitter::EmitIntegerTruncationCheck()`.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1011
+
+ assert(isa<llvm::IntegerType>(SrcTy) && isa<llvm::IntegerType>(DstTy) &&
+ "clang integer type lowered to non-integer llvm type");
----------------
erichkeane wrote:
> This seems like a silly assert, since you did the check above.
If this assertion doesn't hold, we'll (hopefully!) hit an assertion somewhere
down in the [IR] Builder.
I think it would be best to be proactive here.
(Similarly, this is copied verbatim from
`ScalarExprEmitter::EmitIntegerTruncationCheck()`.)
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1030
+ {
+ // At least one of the values needs to have signed type.
+ // If both are unsigned, then obviously, neither of them can be negative.
----------------
erichkeane wrote:
> Does this really need its own scope?
Yeah, they got out of hand here..
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1032
+ // If both are unsigned, then obviously, neither of them can be negative.
+ if (!(SrcSigned || DstSigned))
+ return;
----------------
erichkeane wrote:
> Again, I'd rather we distribute the '!'.
Here - ok.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1044
+
+ assert(!DstType->isBooleanType() && "we should not get here with booleans.");
+
----------------
erichkeane wrote:
> Curious what prevents this?
Right now this was simply copied from
`ScalarExprEmitter::EmitIntegerTruncationCheck()`,
where it made sense (since conversion to bool should be comparison with 0, not
truncation).
I'm not quite sure about booleans here. I think i should just drop it, at least
for now.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1054
+ const char *Name) -> Value * {
+ // Does this Value has signed type?
+ bool VSigned = VType->isSignedIntegerOrEnumerationType();
----------------
erichkeane wrote:
>
> // Is this value a signed type?
That reads strange, but i don't have a better idea.
Repository:
rC Clang
https://reviews.llvm.org/D50250
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
