[PATCH] D28767: Do not verify Dominator tree if it has no roots

[Serge Pavlov via llvm-commits]

2017-01-25 23:20 GMT+07:00 Daniel Berlin <dberlin at dberlin.org>:

> +Chandler.
>
>
> On Wed, Jan 25, 2017 at 7:04 AM, Serge Pavlov <sepavloff at gmail.com> wrote:
>
>> The fact is that sometimes domtree is not available.
>>
>
>
> Then why is there a DominatorTree * to use?
> It's fine for the analysis to be not available. When that happens,it
> should not produce a DominatorTree *.
>
>
`DominatorTree` is not an anaysis. `MachineDominatorTree` is an analysis an
it is available, `getAnalysis<MachineDominatorTree>()` returns not null.


> IE DominatorTree * should be a nullptr you can check against.
>
>

It can be, not should. `DT` is a private field of `MachineDominatorTree`,
there are no requirements how to represent DominatorTree and how to work
with it inside the pass. Using the check for presence of roots is merely
simpler, the patch is more compact.


> If a function is `available_externally`  all codegen passes are skipped,
>> although they are marked as required.
>>
>
> See above :)
>
>
>> MachineDominatorTree is also skipped and there is no domtree at all.
>>
>
> Again, so why is there a DominatorTree * to use?
>

There are no problems to change implementation of `MachineDominatorTree` to
use null pointer as indicator of unavailable domtree. It however doesn't
present any gain, just another way to fetch info that the pass results are
not available.


>
>> Attempt to call `getRoot` for such domtree will obviously cause a crash.
>> There is no way to get 'right' domtree in this case, so prior to using it
>> we must either:
>> 1. Check if corresponding pass was indeed run, or
>> 2. Check the domtree state trying to reveal if it is valid.
>> The first approach was taken in https://reviews.llvm.org/D27190 but it
>> was not accepted. This fix tries to use variant 2.
>>
>
>
>>
>> Just to emphasize a key point: there are cases when domtree is absent
>> because the pass that it creates was not run and this behavior is by design.
>>
>>
> and in those cases, we should  not try to create an invalid datastructure.
> We should create no datastructure, and let people test against the nullptr.
>
> We do not create invalid datastructures, we use particular state of a data
field to deduce that the pass does not have results, `getAnalysis` is
useless in this particular case.

`MachineDominatorTree` is a pass and must obey API conventions,
`getAnalysis` may be called for it and if it returns nullptr, a user knows
that results are absent. `DominatorTree` is an internal datastructure,
users don't work with it directly, they call `verifyDomTree` and it is a
task of implementation to find out if domtree is available. There are no
requirements how to do this.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170126/b2183d1e/attachment.html>