Weddington, Eric wrote: > >> Georg-Johann Lay: >> >> Hi, here is an updated patch. >> >> Some functions are reworked and there is some code clean up. >> >> The test results look good, there are no additional regressions. >> >> The new test cases in gcc.dg/fixed-point pass except some convert-*.c for >> two reasons: >> >> * Some test cases have a loss of precision and therefore fail. One fail is >> that 0x3fffffffc0000000 is compared against 0x4000000000000000 and thus >> fails. Presumably its a rounding error from float. I'd say this is not >> critical. >> >> * PR54330: This leads to wrong code for __satfractudadq and the wrong code >> is already present in .expand. From the distance this looks like a >> middle-end or tree-ssa problem. >> >> The new patch implements TARGET_BUILD_BUILTIN_VA_LIST. Rationale is that >> avr-fixed.md adjust some modes bit these changes are not reflected by the >> built-in macros made by gcc. This leads to wrong code in libgcc because it >> deduces the type layout from these built-in defines. Thus, the respective >> nodes must be patches *before* built-in macros are emit. >> >> The changes to LIB2FUNCS_EXCLUDE currently have no effects, this needs >> http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01580.html which is currently >> under review. >> >> Ok to install? > > Hi Johann, > > I have no objections to the patch, but I think it also best to wait for > Denis to approve as well. > > Based on your analysis of the test case failure you mentioned above, do you > think we need to have some other new test cases for the AVR fixed-point > support?
PR54330 does not look avr-related, and I don't think it makes sense to work around that by cutting down test coverage. That problem reminds me of a middle-end bug where C's undefined signed overflow was applied to IR. This is wrong because signed overflow is only undefined if it comes from the C source but not if it comes from IR optimizers that mess with signed/unsigned/shifts/comparisons/carry etc. on IR. The rounding error is no reason to cut down coverage. The convert tests operate on powers of 2 which all can be represented exactly by float and fixed, so there should not be a rounding error, not even a small one. But maybe it's not a rounding error but just a saturation artifact because a value is saturated to 0xffffffff when converting to/from float. I don't know if there are other values that don't have 2^32-1 in their representation -- I did not track all these errors. The test cases are insanely big and it takes time to follow what's going on with a small target if there is no debugger. What turned out to be unreliable is the out_fixed routine. I fixed the problems I found and tried to add comments to make the code more comprehensible, but there may be more problems. I intend to rewrite that routine from scratch and replace the original version altogether, but it might take some weeks until I have time to return to that. Johann
