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: ↓ 13 Changed 11 years ago by sjoelund.se
comment:2 follow-up: ↓ 11 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: ↓ 12 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: ↓ 6 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.
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)
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.
They are not missing functions: they are partial functions that should never have been added...