[PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
Vincent Lejeune
vljn at ovi.com
Mon Apr 8 10:30:13 PDT 2013
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>
>>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-a-case-to-LiveIntervalAnalysis-HandleMove.patch
Type: application/octet-stream
Size: 8876 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/d398b4be/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-R600-Recompute-schedule-graph-non-order-dependencies.patch
Type: application/octet-stream
Size: 28265 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/d398b4be/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-R600-Use-bottom-up-scheduling-algorithm.patch
Type: application/octet-stream
Size: 9480 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/d398b4be/attachment-0002.obj>
More information about the llvm-commits
mailing list