[llvm-commits] [llvm] r160751 - in /llvm/trunk: include/llvm/Analysis/MemoryBuiltins.h lib/Analysis/MemoryBuiltins.cpp lib/Transforms/Instrumentation/BoundsChecking.cpp test/Instrumentation/BoundsChecking/strings.ll

[Chandler Carruth]

Sorry it's taken me a bit to circle back to this problem...

On Wed, Jul 25, 2012 at 2:34 PM, Nuno Lopes <nunoplopes at sapo.pt> wrote:

> Quoting Benjamin Kramer <benny.kra at gmail.com>:
>
> > On 25.07.2012, at 22:24, Nuno Lopes <nunoplopes at sapo.pt> wrote:
> >
> >> Quoting Duncan Sands <baldrick at free.fr>:
> >>
> >>> Hi Nuno,
> >>>
> >>>> original commit msg:
> >>>> MemoryBuiltins: add support to determine the size of strdup'ed
> >>>> non-constant strings
> >>>
> >>> the dragonegg bots don't use cmake but they broke too:
> >>>
> >>> MemoryBuiltins.cpp:(.text+0x40dc): undefined reference to
> >>> `llvm::EmitStrNLen(llvm::Value*, llvm::Value*, llvm::IRBuilder<true,
> >>> llvm::ConstantFolder, llvm::IRBuilderDefaultInserter<true> >&,
> >>> llvm::TargetData
> >>> const*, llvm::TargetLibraryInfo const*)'
> >>> MemoryBuiltins.cpp:(.text+0x4207): undefined reference to
> >>> `llvm::EmitStrLen(llvm::Value*, llvm::IRBuilder<true,
> llvm::ConstantFolder,
> >>> llvm::IRBuilderDefaultInserter<true> >&, llvm::TargetData const*,
> >>> llvm::TargetLibraryInfo const*)'
> >>
> >> Interesting.. I guess I need to try to build it on another machine.
> >
> > Isn't using lib/Transforms/Utils from lib/Analysis a layering
> > violation (cyclic dependency)?
>
> Well, I really need to access some functions from
> lib/Transforms/Utils. Is there any dependency of lib/Transforms/Utils
> on lib/Analysis?
>

No, you simply cannot depend on lib/Transforms code from within
lib/Analysis. That is a complete violation of the layering design here.

I really don't have a clue on how to solve this problem.. :)
>

I think the fundamental problem is that ObjectSizeOffsetEvaluator isn't an
analysis, and shouldn't be in lib/Analysis at all. From its description:

"Evaluate the size and offset of an object pointed [to] by a value*. May
create code to compute the result at run-time.". An Analysis pass shouldn't
create code. The only consumer of this interface is
lib/Transforms/Instrumentation/BoundsChecking.cpp, I think this class
should move to an anonymous namespace within that instrumentation pass. If
we want to hoist it, it could grow to live in
lib/Transforms/Utils/ObjectSize.cpp or even
lib/Transforms/Utils/MemoryBuiltins.cpp. I would only move it out of the
anonymous namespace if you have a pretty strong idea that others will want
to use the logic. I would only move it to a memory builtin utility file if
you think its likely we'll want more general memory builtin transform
utilities in the future.

Can I make at least the move to the anonymous namespace? I'd like to get
the layering issue fixed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120727/68574d69/attachment.html>