[llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since
Mehdi Amini via llvm-dev
llvm-dev at lists.llvm.org
Wed Feb 10 10:50:57 PST 2016
> On Feb 9, 2016, at 6:43 PM, Duncan P. N. Exon Smith via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>> 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
> too late?
I wonder if it isn't it a bit to much detailed to put this in the release notes? I mean we change the C++ API all the time...
Here is the level of details we went with in 3.7: http://llvm.org/releases/3.7.0/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release
More information about the llvm-dev