[PATCH] D30047: [GlobalISel] Split LowLevelType to a fundamental type in libSupport.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In https://reviews.llvm.org/D30047#689257, @ab wrote:

> In https://reviews.llvm.org/D30047#688544, @dsanders wrote:
>
> > Unfortunately, this approach introduces a circular dependency when LLVM_ENABLE_MODULES=1. The failure was in http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules-2/builds/4942 and was caused by the following dependency loop:
> >
> >   LLVM_Utils -> LLVM_CodeGen_MachineValueType -> LLVM_Utils
>
>
> Oh boy..
>
> > To write it another way, we have these dependencies:
> > 
> > - TableGen -> Support
> > - CodeGen -> Support
> > - Support -> CodeGen
>
> None of these are new, right?


Support -> CodeGen is new. It's the include of MachineValueType.h in LowLevelTypeImpl.h that introduces it.

Before this patch the dependencies were:

- TableGen -> Support
- TableGen -> CodeGen
- CodeGen -> Support

and this patch changed 'TableGen -> CodeGen' into 'TableGen -> Support -> CodeGen'.

> Looking at the log, none of the failures are related to LLT.

LLT is mentioned in the 'While building ...' lines:

  While building module 'LLVM_Utils' imported from /home/buildbot/modules-slave-2/clang-x86_64-linux-selfhost-modules-2/llvm.src/lib/TableGen/StringMatcher.cpp:14:
  While building module 'LLVM_CodeGen_MachineValueType' imported from /home/buildbot/modules-slave-2/clang-x86_64-linux-selfhost-modules-2/llvm.src/include/llvm/Support/LowLevelTypeImpl.h:32:
  In file included from <module-includes>:1:
  /home/buildbot/modules-slave-2/clang-x86_64-linux-selfhost-modules-2/llvm.src/include/llvm/CodeGen/MachineValueType.h:18:10: fatal error: cyclic dependency in module 'LLVM_Utils': LLVM_Utils -> LLVM_CodeGen_MachineValueType -> LLVM_Utils

>> I can see two ways forward:
>> 
>> - We use the original version of https://reviews.llvm.org/D30046 (https://reviews.llvm.org/D30046?id=88737) so that we continue to have the following dependencies:
>>   - TableGen -> Support
>>   - TableGen -> CodeGen
>>   - CodeGen -> Support
>> - We also move the MVT type to Support so we remove the TableGen->CodeGen dependency and are left with:
>>   - TableGen -> Support
>>   - CodeGen -> Support
> 
> How about a lazy variant of this:  move MVT to the Utils module, without changing its path on disk.  It's already in its own special little module anyway.

I'll give it a go. I'll also try removing the MVT dependency since the relevant constructor isn't used very often as far as I can tell. We might be able to switch it for something like getLLTForType().


https://reviews.llvm.org/D30047