[llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since
Duncan P. N. Exon Smith via llvm-dev
llvm-dev at lists.llvm.org
Tue Feb 9 18:43:54 PST 2016
> On 2016-Feb-09, at 12:54, Harris, Kevin <Kevin.Harris at unisys.com> wrote:
> Kevin Harris here, from Unisys. In our application, generating LLVM IR, we have several instances of code that looks like this:
> . . .
> Value* pDS;
> . . .
> auto argIt = pFunc->arg_begin();
> pDS = argIt;
> . . .
> This construct works fine in 3.7, but in LLVM 3.8.0rc2, I’m now getting an error:
> g++-5.2.0 `/home/kharris/dyntrans/llvm-install/bin/llvm-config --cxxflags` -D JITREV="\"4793\"" -D GPPVERSION="\"5.2.0"\" -D LLVMVERSION=38 -D LLVMBUILDVERSION="\"3.8.0"\" -D LLVMBUILDTYPE="\"Debug+Asserts"\" -fno-rtti -Wall -Wextra -Wshadow -c -fmessage-length=0 -fPIC -std=c++11 -o "Debug/dt_llvm.o" "Debug/../dt_llvm.cpp" -I ../MarinerIP -O3 -g3 -m64 -march=core2 -MMD -MP -MF "Debug/dt_llvm.d" -MT "Debug/dt_llvm.d"
> Invoking: g++-5.2.0 Compiler
> Debug/../dt_llvm.cpp: In static member function âstatic llvm::Function* BREGS::selectBasicBreg(Access_t)â:
> Debug/../dt_llvm.cpp:1772:17: error: cannot convert âllvm::ilist_iterator<llvm::Argument>â to âllvm::Value*â in assignment
> pDS = argIt;
You probably want `pDS = &*argIt`, which makes the conversion from a
possibly-not-dereferenceable-iterator to a pointer explicit.
> Evidently, the result of the call to pFunc->arg_begin(); is no longer convertible to a “Value*” type in 3.8. I checked on the type of an Argument, it is still derived from a Value type, so that isn’t the problem. But I discovered that an “ArgumentListType” has indeed changed. In 3.7, in /include/IR/Function.h, this was declared as:
> typedef iplist<Argument> ArgumentListType;
> whereas in LLVM 3.8.0rc2, this is changed to:
> typedef SymbolTableList<Argument> ArgumentListType;
> Rummaging around in the svn log for Function.h, I discovered that this change occurred at svn rev 249602, which you annotated as:
> IR: Create SymbolTableList wrapper around iplist, NFC
> Create `SymbolTableList`, a wrapper around `iplist` for lists that
> automatically manage a symbol table. This commit reduces a ton of code
> duplication between the six traits classes that were used previously.
> As a drive by, reduce the number of template parameters from 2 to 1 by
> using a SymbolTableListParentType metafunction (I originally had this as
> a separate commit, but it touched most of the same lines so I squashed
> I'm in the process of trying to remove the UB in `createSentinel()` (see
> the FIXMEs I added for `ilist_embedded_sentinel_traits` and
> `ilist_half_embedded_sentinel_traits`). My eventual goal is to separate
> the list logic into a base class layer that knows nothing about (and
> isn't templated on) the downcasted nodes -- removing the need to invoke
> UB -- but for now I'm just trying to get a handle on all the current use
> cases (and cleaning things up as I see them).
> Besides these six SymbolTable lists, there are two others that use the
> addNode/removeNode/transferNodes() hooks: the `MachineInstruction` and
> `MachineBasicBlock` lists. Ideally there'll be a way to factor these
> hooks out of the low-level API entirely, but I'm not quite there yet.
> Since this change is annotated as NFC, I’m thinking that the actual cause of my error is some other (earlier) change to the way ArgumentListType is derived, but I haven’t found it yet, so I thought I’d bug you first.
You're looking for r252380. Relevant excerpt:
Note: if you have out-of-tree code, it should be fairly easy to revert
this patch downstream while you update your out-of-tree call sites.
Note that these conversions are occasionally latent bugs (that may
happen to "work" now, but only because of getting lucky with UB;
follow-ups will change your luck). When they are valid, I suggest using
`->getIterator()` to go from pointer to iterator, and `&*` to go from
iterator to pointer.
> Is this change indeed intended as a visible API change to source code generating references to argument list values? If so, can you point me to a description of how I should change our code? Should I bug someone else about this problem? Should this API change be documented in the 3.8.0 release notes?
Release notes is a good idea; somehow I never think of that. Hans, is it
More information about the llvm-dev