[llvm-dev] [RFC] Add DebugLoc parameter in Instruction’s Create() functions

[David Blaikie via llvm-dev]

Might be a lot of functions to change - maybe a wrapper would be easier:

template<typename T>
T *applyDebugLoc(T *t, DebugLoc L) { t->setDebugLoc(L); return t; }

Or change/improve/modify/add setDebugLoc to return the Instruction* so it
could be chained? (NewInst = Foo::Create(...).applyDebugLoc(L); )

On Tue, Aug 7, 2018 at 4:59 AM Anast Gramm via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Many DI-related bugs are caused by missing Debug Location
> in an instruction created in a transformation. Most of the
> time the fix is trivial once you found where the culprit
> instruction is created (https://reviews.llvm.org/D50263).
> Currently, when you create a new Instruction, in order to
> give it DL you have to either use an IRBuilder that is
> previously set to the correct DL or “manually” create
> the instruction via one of it’s Create() routines and then
> call `setDebugLoc()` to it.
>
> I propose the addition of a DebugLoc parameter
> in the *::Create() instruction constructors.
> This could be in the form of a pure DebugLoc
> variable or possibly an `Instruction *InheritLocationFromInst` one
>
> Some pros of this idea are:
>  - Easier to create instructions with debug location.
>  - It will make the code more readable by eliminating
>  the many `NewInst->setDebugLoc(OldInst->getDebugLoc)` calls.
>  - It will incentivize people to think about the DebugLoc of
>  their newly created instruction and where it should come from.
>
> As the Create() functions are widely used, and sometimes
> location data could be out of scope when the Instruction
> is created, I think the best approach to this is to
> introduce the new field as optional that defaults to
> empty DL (as it is now) to avoid huge refactoring,
> and to allow more time for testing individual changes.
> Refactoring could then be done in steps and the
> parameter could become mandatory in some ::Create()
> constructors as we judge fit.
>
> With that in mind here are some cons:
>  - Incomplete transition of APIs.
>  - Lack of infrastructure to check that the locations
>  supplied to *::Create are correct.
>
> What do you think?
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180807/a5443b7e/attachment.html>