Please do not reply to this email- if you want to comment on the bug, go to the URL shown below and enter your comments there.
Changed by [EMAIL PROTECTED] http://bugzilla.ximian.com/show_bug.cgi?id=75738 --- shadow/75738 2005-08-09 07:05:50.000000000 -0400 +++ shadow/75738.tmp.15090 2005-08-19 15:42:37.000000000 -0400 @@ -1,12 +1,12 @@ Bug#: 75738 Product: Mono: Runtime Version: 1.1 OS: unknown OS Details: amd64 pld linux -Status: NEW +Status: ASSIGNED Resolution: Severity: Unknown Priority: Major Component: JIT AssignedTo: [EMAIL PROTECTED] ReportedBy: [EMAIL PROTECTED] @@ -81,6 +81,107 @@ It happens both on x86 and amd64. ------- Additional Comments From [EMAIL PROTECTED] 2005-08-07 15:13 ------- This seems to be yet another remove_block_if_useless bug. massi, could you look at it ? + +------- Additional Comments From [EMAIL PROTECTED] 2005-08-19 15:42 ------- + +This bug, after all, does not seem to be related to +"remove_block_if_useless". +To be *really* sure, I #ifdeffed it out, and the bug persists. + +The real problem seems that "move_basic_block_to_end" does not +"keep connected" two consecutive BBs. + +It can be seen this way: the following IL (part of the test) + + IL_000d: brfalse IL_0017 + IL_0012: br IL_0021 +// This is BB 6 + IL_0017: ldstr "ble" + IL_001c: call void class +[mscorlib]System.Console::WriteLine(string) +// This is BB 7 + IL_0021: ldstr "" + IL_0026: newobj instance void class +[mscorlib]System.Exception::.ctor(string) + IL_002b: throw + +becomes this sequence in the JIT + + (bne.un[B7B6] (compare (ldind.u1 regoffset[-0x9(%ebp)]) iconst[0])) +DUMP BLOCK 6: + (outarg iconst[136716224]) + voidcall[WriteLine] +DUMP BLOCK 1: +DUMP BLOCK 7: + (outarg iconst[136045336]) + (stind.ref regoffset[-0x4(%ebp)] call[mono_object_new_fast]) + (outarg iconst[136728544]) + (outarg (ldind.ref regoffset[-0x4(%ebp)])) + voidcall[.ctor] + (throw (ldind.ref regoffset[-0x4(%ebp)])) + not_reached + +Don't care about the BB with a CEE_BR at IL_0012, it is not the +point, "remove_block_if_useless" safely eliminates it, but in any +case also other branch optimizations take it away, logged like this: + +cbranch2 to branch triggered 5 -> (9) 7 (0x38) +nullify block triggered 9 + +The real issue is that BB 1 is inserted between BB 6 and BB 7 because +BB 7 is "out_of_line", and so gets moved to the end. +However, BB 6 *requires* to be followed by BB 7 (the test should 1st +print "ble" and then throw the exception). +The "move_basic_block_to_end" function looks for the previous BB, but +just to link it to the new next BB, without checking if a CEE_BR must +be added to preserve the fact that the previous BB expected to "fall +through" into its next (which is being moved at the end). + +So, the bug is the fact that BB 1 (which on x86 emits the sequence +"leave, ret") is executed just after BB6, so that BB7 is skipped. + +Obviously the fix would be to check if a CEE_BR must be added to the +"previous BB" (in this example BB 6), and adding it if needed (in this +case jumping to BB 7). + +The reason why I'm writing this long comment is that I can imagine one +case where this cannot be done: if the "previous BB" (think BB 6) ends +with a CEE_SWITCH. +In this case the fall through behavior is crucial (it is the "default" +of the switch), but we cannot add a CEE_BR after a CEE_SWITCH in the +same BB. +So, to preserve the fall through, a new BB should be created and put +just after the "previous BB", and the CEE_BR should be put in this new +BB. + +What bothers me is that creating a BB after "cfg->bblocks" has been +allocated and "df_visit" has been called is (IMHO) a nightmare. + +Luckily, optimize_branches is called two times: one before df_visit, +and the other after mono_ssa_remove. +One could assume that the need to move out of band BBs emerges just +in the 1st call to optimize_branches, where creating BBs is still an +easy thing. + +So, my plan is: +- Add a gboolean parameter to optimize_branches, stating if BBs can + be created (let's call it "can_create_blocks"). + Pass TRUE in the 1st call, FALSE in the 2nd. +- Inside optimize_branches, do one of the following: + [a] Modify the "cbranch to throw block triggered" code to check if + the optimization can be applied (if the "previous BB" ends with + a CEE_SWITCH, can_create_blocks must be TRUE). + [b] If can_create_blocks is FALSE, skip the "cbranch to throw block + triggered" section completely. +- In any case add the missing CEE_BR :-) + +Now, my questions are: +Is this analysis correct? +Do you prefer option a or b? + +Ciao, + Massi + _______________________________________________ mono-bugs maillist - [email protected] http://lists.ximian.com/mailman/listinfo/mono-bugs
