onox / orka

The OpenGL 4.6 Rendering Kernel in Ada 2022
https://orka-engine.netlify.com/
Apache License 2.0
61 stars 2 forks source link

Use short circuit operators #86

Closed pjljvandelaar closed 1 year ago

pjljvandelaar commented 1 year ago

Dear Developers,

Within orka short circuits operators are not always used where possible.

See attached patch for places where a short circuit operator should be used.

Greetings, Pierre

Short_Circuit.zip

onox commented 1 year ago

I don't think it improves the readability and the non-short circuit operators may give the compiler more opportunities for optimizing the code.

pjljvandelaar commented 1 year ago

Hi Onox,

Your statement that the compiler has more opportunities for optimizing the code with non short circuit operators is invalid.

See e.g. https://www.adaic.org/resources/add_content/docs/95style/html/sec_5/5-5-5.html and

In the absence of short-circuit forms, Ada does not provide a guarantee of the order of expression evaluation, nor does the language guarantee that evaluation of a relational expression is abandoned when it becomes clear that it evaluates to False (for and) or True (for or).

and https://perso.telecom-paristech.fr/pautet/Ada95/chap04.htm

The way the expressions are written in lines 26 through 29, all of the expressions will be evaluated when the statement is executed. If, in the case of line 26, the value of Index is not 12, then the final result will be FALSE no matter what the rest of the expressions are and it would be a waste of time to evaluate them. Ada will continue blindly across the entire line evaluating all of the expressions and wasting time since it should know the final result based on the first comparison. There is however, a way to tell it to stop as soon as it knows the final answer, through use of the short circuit operators.
pjljvandelaar commented 1 year ago

To find places where membership tests are preferred, our software only matches with pattern that use short-circuit operators.

I noticed that your code can benefit from membership tests at some locations, see e.g. orka/src/orka/implementation/orka-frame_graphs.adb

- if Resource.Input_Mode /= Not_Used and Resource.Input_Mode /= Mode then
+ if Resource.Input_Mode not in Not_Used | Mode then 
onox commented 1 year ago

and it would be a waste of time to evaluate them. Ada will continue blindly across the entire line evaluating all of the expressions

This information seems to be from the late 90s. Is this still the case with a modern version of GNAT? I would think that by using and and or you give the compiler more freedom to optimize.

see e.g. orka/src/orka/implementation/orka-frame_graphs.adb

Did not know I could combine an enum literal and a variable in membership test :open_mouth: