ro added a comment. In D91607#3285825 <https://reviews.llvm.org/D91607#3285825>, @efriedma wrote:
> 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. Understood: this way you don't rely on native builds and can test SPARC V8 structure return which isn't exercised by the sanitzer tests. >>> 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. Cool: I'd been struggling with `CreateConstInBoundsByteGEP` which seemed what I need, but had me fighting with conversion between `llvm::Value *` and `Address`. 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
