[PATCH] D76539: [CodeGenPrepare] Delete intrinsic call to llvm.assume to enable more tailcall

Guozhi Wei via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 24 12:54:09 PDT 2020


Carrot marked an inline comment as done.
Carrot added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:1989-1990
     }
     case Intrinsic::objectsize:
       llvm_unreachable("llvm.objectsize.* should have been lowered already");
     case Intrinsic::is_constant:
----------------
arsenm wrote:
> Carrot wrote:
> > arsenm wrote:
> > > Carrot wrote:
> > > > arsenm wrote:
> > > > > Carrot wrote:
> > > > > > arsenm wrote:
> > > > > > > I would expect like object size/is_constant this would have been lowered already? I do disagree with using unreachable to error on this case though
> > > > > > This is not in my patch.
> > > > > > Do you want me to delete these 2 cases in the same patch?
> > > > > I know, but I would expect this case to be handle the same way. These should also not be deleted, just upgraded to an error that won't be deleted in a release build
> > > > objectsize and is_constant are lowered at LowerConstantIntrinsics, Intrinsic::assume does not generate instructions, so was simply ignored at SelectionDAG. Do you mean delete it at LowerConstantIntrinsics?
> > > > 
> > > > Also does "just upgraded to an error that won't be deleted in a release build" mean Ctx.emitError()?
> > > > 
> > > > thanks!
> > > That or report_fatal_error
> > Also delete Intrinsic::assume in LowerConstantIntrinsics?
> It's not handled there as you said, so I'm not sure what you're asking?
Sorry for my bad English :(

Is this version OK now?


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

https://reviews.llvm.org/D76539





More information about the llvm-commits mailing list