sameerds added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:386-387
+ } else {
+ auto IntTy = dyn_cast<IntegerType>(Args[i]->getType());
+ if (IntTy && IntTy->getBitWidth() == 32)
+ WhatToStore.push_back(
----------------
vikramRH wrote:
> arsenm wrote:
> > vikramRH wrote:
> > > arsenm wrote:
> > > > isIntegerTy(32).
> > > >
> > > > I also do not understand why only 32-bit integers would be promoted to
> > > > 64-bit (or why this would be zext). This doesn't match varargs ABI
> > > > handling, where everything smaller would be promoted to i32.
> > > Right, But default promotions would have happened already, this
> > > additional promotion is due to runtime processing requirement
> > Is this documented somewhere? If it's promote everything to i64, I'd prefer
> > to handle all types rather than special casing 32
> That's precisely what the function "fitArgInto64Bits()" did. We eliminated it
> due to some redundant instructions generated. int32 was the only case (apart
> from float16 now) that required this special casting.
I think the problem is that the check for "isIntergerTy(32)" makes it look like
it is some sort of special case. What we really mean is that if the type is an
integer with width less than 64 bits, then zext it with padding to fit the
64-bit store into the buffer. We can do that with just "isIntegerTy()".
Separately, we know from auto promotion of varargs that the integer can only be
of size 32 or 64. We can assert that instead of assuming it in the code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150427/new/
https://reviews.llvm.org/D150427
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
