[PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
Vincent Lejeune
vljn at ovi.com
Sun Apr 14 07:03:52 PDT 2013
Hi Jakob,
can I have a review of the first patch ? Apparently It doesn't bring regressions,
and I think I handled Live out value the way you mentionned in your previous mail.
Vincent
----- Mail original -----
> De : Vincent Lejeune <vljn at ovi.com>
> À : Andrew Trick <atrick at apple.com>
> Cc : Jakob Stoklund Olesen <stoklund at 2pi.dk>; Commit Messages and Patches for LLVM <llvm-commits at cs.uiuc.edu>
> Envoyé le : Lundi 8 avril 2013 19h30
> Objet : Re: [PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
>
>
> Hi,
>
> The patches didn't apply cleanly due to recent commit, here are rebased
> versions
> I ran lnt with clang @179017, llvm @179014 (+ patches), llvm-test-suite
> 179022
>
> Apparently there is no regression, but I still have two failling tests,
> whether I apply or not my patches :
> --- Tested: 988 tests --
> FAIL: MultiSource/Applications/ClamAV/clamscan.execution_time (495 of
> 988)
> FAIL: MultiSource/Benchmarks/Olden/voronoi/voronoi.execution_time (496
> of 988)
>
> Vincent
>
>
>
> ----- Mail original -----
>> De : Andrew Trick <atrick at apple.com>
>> À : Vincent Lejeune <vljn at ovi.com>
>> Cc : Jakob Stoklund Olesen <stoklund at 2pi.dk>; Commit Messages and
> Patches for LLVM <llvm-commits at cs.uiuc.edu>
>> Envoyé le : Mardi 12 mars 2013 21h25
>> Objet : Re: [PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
>>
>>
>> On Mar 12, 2013, at 10:54 AM, Vincent Lejeune <vljn at ovi.com> wrote:
>>
>>> I ran with both options and get no regressions.
>>>
>>> The execution_time tests that fail with and without my patches are the
>
>> following ones by the way :
>>>
>>> --- Tested: 986 tests --
>>> FAIL: MultiSource/Applications/Burg/burg.execution_time (494 of 986)
>>> FAIL: MultiSource/Applications/ClamAV/clamscan.execution_time (495 of
> 986)
>>> FAIL: MultiSource/Applications/lemon/lemon.execution_time (496 of 986)
>>> FAIL:
>>
> MultiSource/Benchmarks/MiBench/automotive-bitcount/automotive-bitcount.execution_time
>
>> (497 of 986)
>>> FAIL:
>> MultiSource/Benchmarks/MiBench/telecomm-FFT/telecomm-fft.execution_time
> (498 of
>> 986)
>>> FAIL: MultiSource/Benchmarks/Olden/voronoi/voronoi.execution_time (499
> of
>> 986)
>>> FAIL: MultiSource/Benchmarks/Ptrdist/anagram/anagram.execution_time
> (500 of
>> 986)
>>> FAIL: SingleSource/Benchmarks/BenchmarkGame/puzzle.execution_time (501
> of
>> 986)
>>
>> Thanks Vincent. I don't see these failures. Your test configuration
> must be
>> different. If they fail with ToT llvm and test-suite (make sure to update
> your
>> tests periodically), then it's worth filing a bug.
>>
>> I'm usually testing (not benchmarking) with:
>>
>> 'make' 'SMALL_PROBLEM_SIZE=1' 'report'
>> 'report.csv' '-j8' 'TEST=simple'
> 'TARGET_FLAGS=-arch
>> x86_64 -Wno-error=implicit-function-declaration' 'OPTFLAGS=-O3'
>
>> 'ARCH=x86_64' 'CC_UNDER_TEST_TARGET_IS_X86_64=1'
>> 'CLANGPATH=/b/test/fix-x86/bin/clang'
>> 'CLANGXXPATH=/b/test/fix-x86/bin/clang++'
> 'LLVMCC_OPTION=clang'
>> 'CC_UNDER_TEST_IS_CLANG=1' 'DISABLE_CBE=1'
>> 'DISABLE_JIT=1' 'ENABLE_PARALLEL_REPORT=1'
>> 'ENABLE_HASHED_PROGRAM_OUTPUT=1' 'USE_REFERENCE_OUTPUT=1'
>>
>> If you're not using reference output, it could actually be a bug with
> your
>> reference compiler.
>>
>> -Andy
>>
>>> ----- Mail original -----
>>>> De : Andrew Trick <atrick at apple.com>
>>>> À : Vincent Lejeune <vljn at ovi.com>
>>>> Cc : Jakob Stoklund Olesen <stoklund at 2pi.dk>; Commit
> Messages and
>> Patches for LLVM <llvm-commits at cs.uiuc.edu>
>>>> Envoyé le : Mardi 12 mars 2013 17h18
>>>> Objet : Re: [PATCH] Add a case to
> LiveIntervalAnalysis::HandleMoveUp
>>>>
>>>>
>>>> On Mar 12, 2013, at 7:26 AM, Vincent Lejeune <vljn at ovi.com>
>> wrote:
>>>>
>>>>> Hi Jacob and Andrw,
>>>>>
>>>>> I reworked my patch so that now it does not change the value
> of a
>> live out
>>>> value. It also keep live in value.
>>>>> The 2 others patches enable mov of instructions that access
>> sub-register
>>>> (needed to test the feature).
>>>>>
>>>>> I added some tests using -verify-misched in a previous patch :
>
>>>>>
>>>>
>>
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130311/167949.html
>>>>> They come from piglit (the test suite we use for opengl) and
> failed
>> often
>>>> when I was working on handlemove of subregs,
>>>>> I think they would be good candidate for unit testing the
> feature.
>>>>>
>>>>> There are also 4 unit tests in the third patches, these are
> simple
>> modules
>>>> which generated not optimal code
>>>>> without recomputing the schedule graph ; the tests check now
> that
>> they do.
>>>>>
>>>>> Thank for your help in getting these patches in a proper form
>> (although I
>>>> think they need reviews) and for your patience.
>>>>> Vincent
>>>>
>>>> Thanks Vincent,
>>>>
>>>> The scheduler patches look great. We can find ways to reduce
> compile
>> time of
>>>> recomputing dependencies eventually, feel free to file a bug if
>> it's a
>>>> problem.
>>>>
>>>> Jakob should review the handleMoveUp/Down logic. To verify I
> suggest
>> running the
>>>> llvm test-suite with
>>>>
>>>> -enable-misched -verify-misched -misched=shuffle
>>>>
>>>> which I think you've done already. But additionally, to cover
> both
>>>> handleMoveUp/Down as much as possible, you can add the options
>>>>
>>>> -misched-topdown
>>>>
>>>> and
>>>>
>>>> -misched-bottomup
>>>>
>>>> -Andy
>>>>
>>>>> ----- Mail original -----
>>>>>> De : Jakob Stoklund Olesen <stoklund at 2pi.dk>
>>>>>> À : Vincent Lejeune <vljn at ovi.com>
>>>>>> Cc : Andrew Trick <atrick at apple.com>; Commit
> Messages and
>> Patches
>>>> for LLVM <llvm-commits at cs.uiuc.edu>
>>>>>> Envoyé le : Mercredi 6 mars 2013 23h11
>>>>>> Objet : Re: [PATCH] Add a case to
>> LiveIntervalAnalysis::HandleMoveUp
>>>>>>
>>>>>>
>>>>>> On Mar 6, 2013, at 2:01 PM, Vincent Lejeune
>> <vljn at ovi.com> wrote:
>>>>>>
>>>>>>> Hi Jakob,
>>>>>>>
>>>>>>>>
>>>>>>>> Did you read my previous mail about the problem
> with
>> live-out
>>>> values?
>>>>>> You
>>>>>>>> didn't address the problem.
>>>>>>>
>>>>>>> I actually don't understand what should be done
> for
>> live out
>>>> values.
>>>>>>> For instance if I have the following LiveInterval :
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
> [48r,64r:2)[64r,80r:3)[80r,96B:4)[96B,336r:0)[448B,640r:0)[640r,672r:1)[672r,688r:5)[688r,704r:6)[704r,816B:7)
>>>>>>> With block boundaries :
>>>>>>> [0B;80B][96B;240B][256B;432B][448B;800B]
>>>>>>> My reasoning is the following :
>>>>>>> LiveRange that represents live-out value in the first
> bloc
>> is
>>>> [80r;96B] ;
>>>>>>> If I move 80r to 8B for instance, I'm expecting
> the the
>> first
>>>> three
>>>>>> LiveRange to be :
>>>>>>> [8r,48r:4)[48r,64r:2)[64r,96B:3)
>>>>>>> ie val 3 is becoming the new live-out value, that ends
>
>> where value
>>>> 4 ended
>>>>>> before.
>>>>>>
>>>>>> The point is that the LiveInterval representation of
> liveness
>> is just a
>>>>
>>>>>> compression method. A single LiveRange object can span
> multiple
>> basic
>>>> blocks,
>>>>>> but for values that are live in more than one basic block,
>
>> there is no
>>>> guarantee
>>>>>> that it only appears in a single LiveRange object. That
> depends
>> on the
>>>> block
>>>>>> layout which you can't make assumptions about.
>>>>>>
>>>>>> To avoid scanning the whole LiveInterval looking for
> instances
>> of the
>>>> escaped
>>>>>> value, I would suggest that you change your code so the
> value
>> number
>>>> escaping
>>>>>> the basic block doesn't change.
>>>>>>
>>>>>> That would mean that the escaping value number is now
> defined
>> by a
>>>> different
>>>>>> instruction - the one that is now the last def in the
> block.
>>>>>>
>>>>>> /jakob
>>>>>
>>>>
>>
> <0001-Add-a-case-to-LiveIntervalAnalysis-HandleMove.patch><0003-R600-Recompute-schedule-graph-non-order-dependencies.patch><0002-R600-Use-bottom-up-scheduling-algorithm.patch>
>>>>
>>
>
More information about the llvm-commits
mailing list