Opened 11 years ago

Last modified 7 years ago

#2609 new defect

BackendDAE modules "removedUnusedFunction" hides other bugs.

Reported by: wbraun Owned by: probably noone
Priority: normal Milestone: Future
Component: Backend Version: trunk
Keywords: Cc: niklwors, lochel

Description (last modified by wbraun)

The module postOptModule "removedUnusedFunction" should be neutral in terms of functionality to our testsuite, but it isn't.
Following results when it's deactivated:

Failed tests:
        ./openmodelica/cppruntime/libraries/msl32/Modelica.Electrical.QuasiStationary.SinglePhase.Examples.ParallelResonance.mos
        ./openmodelica/cppruntime/libraries/msl32/Modelica.Mechanics.MultiBody.Examples.Elementary.DoublePendulum.mos
        ./openmodelica/cppruntime/libraries/msl32/Modelica.Mechanics.MultiBody.Examples.Elementary.RollingWheelSetDriving.mos
        ./openmodelica/debugDumps/optdaedump.mos // doesn't matter
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Air.DryAirNasa.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.Air.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.Nitrogen.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGas.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGasFixedComposition.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.IdealSteam.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97OnePhase_ph.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97_pT.mos
        ./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97_ph.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.AST_BatchPlant.Test.TankWithEmptyingPipe1.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.AST_BatchPlant.Test.TankWithEmptyingPipe2.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.AST_BatchPlant.Test.TanksWithEmptyingPipe1.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.AST_BatchPlant.Test.TanksWithEmptyingPipe2.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.AST_BatchPlant.Test.TwoTanks.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.BranchingDynamicPipes.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.TraceSubstances.RoomCO2.mos
        ./simulation/libraries/msl32/Modelica.Fluid.Examples.TraceSubstances.RoomCO2WithControls.mos
        ./simulation/modelica/algorithms_functions/ForIterator2.mos
        ./simulation/modelica/hpcom/Modelica.Fluid.Examples.BranchingDynamicPipes.mos // duplicated
24 of 2508 failed

So actually this are only 22 failing test, but as far as I see this are only 4 different issues. So will create separated tickets for them.
For:

./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Air.DryAirNasa.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.Air.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.Nitrogen.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGas.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGasFixedComposition.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.IdealSteam.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97OnePhase_ph.mos
./simulation/libraries/msl31/media/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97_pT.mos
./simulation/libraries/msl31/media

the issue is:
State by sjoelund.se:

They are not missing functions: they are partial functions that should never have been added...

Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c: In function ‘omc_Modelica_Media_Interfaces_PartialMedium_specificEnthalpy__pTX’:
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4883: error: ‘Modelica_Media_Interfaces_PartialMedium_setState__pTX_rettype’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4883: error: expected ‘;’ before ‘tmp2’
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4884: error: ‘Modelica_Media_Interfaces_PartialMedium_specificEnthalpy_rettype’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4884: error: expected ‘;’ before ‘tmp3’
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4889: error: ‘tmp2’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4890: error: ‘tmp3’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c: In function ‘omc_Modelica_Media_Interfaces_PartialMixtureMedium_specificEnthalpy__pTX’:
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4908: error: ‘Modelica_Media_Interfaces_PartialMixtureMedium_setState__pTX_rettype’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4908: error: expected ‘;’ before ‘tmp2’
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4909: error: ‘Modelica_Media_Interfaces_PartialMixtureMedium_specificEnthalpy_rettype’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4909: error: expected ‘;’ before ‘tmp3’
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4914: error: ‘tmp2’ undeclared (first use in this function)
Modelica.Media.Examples.Tests.MediaTestModels.Air.MoistAir_functions.c:4915: error: ‘tmp3’ undeclared (first use in this function)

For:

./simulation/libraries/msl32/Modelica.Fluid.Examples.BranchingDynamicPipes.mos
./simulation/libraries/msl32/Modelica.Fluid.Examples.TraceSubstances.RoomCO2.mos
./simulation/libraries/msl32/Modelica.Fluid.Examples.TraceSubstances.RoomCO2WithControls.mos

it seems that something in instantiation of function with constants is going wrong, since "_TinK", "_poly_Cp" and "_poly_rho" are constants in package.

Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c: In function 'omc_Modelica_Media_Incompressible_TableBased_h__T__der':
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8790: error: '_TinK' undeclared (first use in this function)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8790: error: (Each undeclared identifier is reported only once
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8790: error: for each function it appears in.)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8800: error: '_poly_Cp' undeclared (first use in this function)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c: In function 'omc_Modelica_Media_Incompressible_TableBased_h__T':
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8829: error: '_TinK' undeclared (first use in this function)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8849: error: '_poly_Cp' undeclared (first use in this function)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c: In function 'omc_Modelica_Media_Incompressible_TableBased_h__pT':
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8892: error: '_TinK' undeclared (first use in this function)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8912: error: '_poly_Cp' undeclared (first use in this function)
Modelica.Fluid.Examples.BranchingDynamicPipes_functions.c:8926: error: '_poly_rho' undeclared (first use in this function)

For ./simulation/modelica/algorithms_functions/ForIterator2.mos we have some code generation issues with arrays. But I have no idea what kind of issue the cppruntime tests have.

Does anybody something about that issue?
I hope it was not the propose of this module to fix this issue;)


Change History (21)

comment:1 follow-up: Changed 11 years ago by sjoelund.se

They are not missing functions: they are partial functions that should never have been added...

comment:2 follow-up: Changed 11 years ago by adrpo

I think removedUnusedFunction phase should always be run at the start of the back-end.
Maybe a better solution would be to do this before the back-end, even before we dump the DAE.
I don't think is that hard to implement. Any takers? :)

We should do this at least until I fix the damn front-end as they will disappear after that.

comment:3 Changed 11 years ago by lochel

Should not be that hard to do. I can handle it after the Modelica conference. Or are there other volunteers?

comment:4 follow-up: Changed 11 years ago by adrpo

Basically DAEUtil.traverseDAE + a function gets a DAE.CALL and returns the updated function tree. We'll see if I get some time this week to do it.

comment:5 follow-up: Changed 11 years ago by sjoelund.se

It's still a useful phase because it removes a lot of code when compiling C-code if there was inlining.

comment:6 in reply to: ↑ 5 Changed 11 years ago by lochel

Replying to sjoelund.se:

It's still a useful phase because it removes a lot of code when compiling C-code if there was inlining.

Well, shouldn't the inline-module remove unused functions after they get inlined?

comment:7 Changed 11 years ago by adrpo

It's not possible to remove the function when you inlined one call. It might be used someplace else where it cannot be inlined. Inlining depends on the function call arguments. So after you inline, some functions might be possible to remove, some might not.

Last edited 11 years ago by adrpo (previous) (diff)

comment:8 Changed 11 years ago by sjoelund.se

Why? Some of the calls might not be inlined. Like LateInline called from both inside function and in model context.

comment:9 Changed 11 years ago by sjoelund.se

And to make things worse: Simplify can also remove function calls, like 0*f(x)

comment:10 Changed 11 years ago by lochel

That's true.

comment:11 in reply to: ↑ 2 Changed 11 years ago by wbraun

Replying to adrpo:

I think removedUnusedFunction phase should always be run at the start of the back-end.
Maybe a better solution would be to do this before the back-end, even before we dump the DAE.
I don't think is that hard to implement. Any takers? :)

No, we can't move it to the end of the Front-End. It's correct to do it at the end of the back-end, since we might use functions within IndexReduction which are not used before(derivative annotation), also it's for jacobian, we might use derivative annotations.

comment:12 in reply to: ↑ 4 Changed 11 years ago by wbraun

  • Priority changed from high to normal

Replying to adrpo:

Basically DAEUtil.traverseDAE + a function gets a DAE.CALL and returns the updated function tree. We'll see if I get some time this week to do it.

There is really no need to hurry. I just wanted to make a note on the issues that were covered by that module, the module it self is note that bad.

comment:13 in reply to: ↑ 1 Changed 11 years ago by wbraun

  • Description modified (diff)
Last edited 11 years ago by wbraun (previous) (diff)

comment:14 Changed 10 years ago by sjoelund.se

  • Milestone changed from 1.9.1 to 1.9.2

This ticket was not closed for 1.9.1, which has now been released. It was batch modified for milestone 1.9.2 (but maybe an empty milestone was more appropriate; feel free to change it).

comment:15 Changed 10 years ago by sjoelund.se

  • Milestone changed from 1.9.2 to 1.9.3

Milestone changed to 1.9.3 since 1.9.2 was released.

comment:16 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.3 to 1.9.4

Moved to new milestone 1.9.4

comment:17 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.4 to 1.9.5

Milestone pushed to 1.9.5

comment:18 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.5 to 1.10.0

Milestone renamed

comment:19 Changed 8 years ago by sjoelund.se

  • Milestone changed from 1.10.0 to 1.11.0

Ticket retargeted after milestone closed

comment:20 Changed 8 years ago by sjoelund.se

  • Milestone changed from 1.11.0 to 1.12.0

Milestone moved to 1.12.0 due to 1.11.0 already being released.

comment:21 Changed 7 years ago by casella

  • Milestone changed from 1.12.0 to Future

The milestone of this ticket has been reassigned to "Future".

If you think the issue is still valid and relevant for you, please select milestone 1.13.0 for back-end, code generation and run-time issues, or 2.0.0 for front-end issues.

If you are aware that the problem is no longer present, please select the milestone corresponding to the version of OMC you used to check that, and set the status to "worksforme".

In both cases, a short informative comment would be welcome.

Note: See TracTickets for help on using tickets.