efriedma added a comment.

In D91607#3283350 <https://reviews.llvm.org/D91607#3283350>, @ro wrote:

> In D91607#3280808 <https://reviews.llvm.org/D91607#3280808>, @efriedma wrote:
>
>> Testcase?
>
> I thought the 3 testcases adjusted in D91608 
> <https://reviews.llvm.org/D91608> to use `__builtin_extract_return_addr` and 
> fixed by this patch were enough.  Otherwise, should I just augment 
> `clang/test/CodeGen/builtins-sparc.c` or better create a new test?

I'd prefer to have coverage in the clang regression tests, so developers can 
catch regressions and easily check the expected codegen.  builtins-sparc.c is 
fine.

>> Do you need to ptrtoint/inttoptr?  I would expect that the address is an 
>> `i8*`, so you can just GEP an appropriate number of bytes.
>
> TBH, I know practically nothing about LLVM codegen, so I rely heavily on 
> guidance.  IIRC this patch was developed by searching for similar code in 
> `TargetInfo.cpp` and modifying it until it did what I needed.  Is this the 
> place to read on GEP <https://llvm.org/docs/GetElementPtr.html>?

That's a good starting place for understanding the complexity of GEP... but you 
don't need that much here. Here, we just want to pass a single index; that's 
equivalent to pointer addition in C.

You should be able to just drop the calls to CreatePtrToInt and CreateIntToPtr, 
and replace CreateAdd with CreateGEP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91607

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to