[LLVMdev] Tablegen problem populating TSFlags

[Joe Matarazzo]

What do you think of the following patch? It's a very small change and
fixes the problem. I think it's innocuous since resolveReferences()
should nominally return (*this) if there's nothing to resolve, so
there's no reason not to run it at the start of
FieldInit::resolveReferences().

When it runs, NewRec != Rec and we return a new FieldInit. Subsequent
calls resolve the OpInit and eventually we get the Bits we want.

Joe

On Thu, Jan 31, 2013 at 2:47 PM, Joe Matarazzo <joe.matarazzo at gmail.com> wrote:
> Jakob, I think this exactly what's happening. I debugged the
> resolveReferences for the ADD down into the resolve of TSFlags. It
> calls VarInit::getFieldInit for the "Val" field of "foo". The code is:
>
> Init *VarInit::getFieldInit(Record &R, const RecordVal *RV,
>                             const std::string &FieldName) const {
>   if (isa<RecordRecTy>(getType()))
>     if (const RecordVal *Val = R.getValue(VarName)) {
>       if (RV != Val && (RV || isa<UnsetInit>(Val->getValue())))
>         return 0;
>       Init *TheInit = Val->getValue();
>       assert(TheInit != this && "Infinite loop detected!");
>       if (Init *I = TheInit->getFieldInit(R, RV, FieldName))  // ***
>         return I;
>       else
>         return 0;
>     }
>   return 0;
> }
>
> The line marked with *** doesn't evaluate TheInit as a TernOpInit
> correctly. It goes to the Init base class of getFieldInit and returns
> 0.
>
> Do we need to add a getFieldInit method on TernOpInit to evaluate the
> !if() and return the requested field of the result ("Val" in this
> case) ?
>
> On Thu, Jan 31, 2013 at 9:35 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>>
>> On Jan 31, 2013, at 9:27 AM, Sean Silva <silvas at purdue.edu> wrote:
>>
>>> An extra call to resolveReferences after setting the value in the
>>> `let` does fix this, but I'm not sure that it is the right fix. The
>>> attached hackish patch seems to fix up a reduced version of the test
>>> case you gave here (I haven't run a full battery of tests on it, so it
>>> might cause other failures).
>>>
>>> Jakob, any idea about what the "right" fix is here?
>>
>> Sorry, no.
>>
>> I do recall seeing similar problems in the past where the ternary operators weren't properly evaluated, but left as TernOpInits in the final Records.
>>
>> I don't know of any use case where that would make sense, particularly since other operators get evaluated. Are they even in use in the tree?
>>
>> /jakob
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: resolveFieldInit.patch
Type: application/octet-stream
Size: 509 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130131/4f8ccd3c/attachment.obj>