search for: expandshiftwithunknownamountbit

Duncan, It seems that the code you pasted came from the function ExpandShiftByConstant and indeed it looks correct. In my example I used 6 as the shift amount but forgot to mention that it's stored in a register. Otherwise ExpandShiftWithUnknownAmountBit wouldn't get called. Below is the execution of DAGTypeLegalizer::ExpandIntRes_Shift() using my example showing how ExpandShiftWithUnknownAmountBit gets called. My email is about the logic of this function. if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(N->getOperand(1))) { <== F...

...t; It seems that the code you pasted came from the function > ExpandShiftByConstant and indeed it looks correct. In my example I used 6 > as the shift amount but forgot to mention that it's stored in a register. I see, sorry I didn't read your email more carefully. It does look like ExpandShiftWithUnknownAmountBit is bogus - at a glance it looks like the Cmp condition is inverted. I may have time to take a closer look tomorrow. Best wishes, Duncan.

...egers to my target. I'm using > LLVM to decompose the 64-bit integer operations by using 32-bit registers > wherever possible and emulating support where not. When looking at the bit > shift decomposition I saw what seems to be a bug in the implementation. The > affected function is ExpandShiftWithUnknownAmountBit in > LegalizeIntegerTypes.cpp. Below is the original code and the proposed fix. > Could someone please review the changes? If they are correct how do I go > about submitting a patch? can you please describe the problem and how you fix it (in words, not code). Thanks, Duncan.

...pport for 64-bit integers to my target. I'm using LLVM to decompose the 64-bit integer operations by using 32-bit registers wherever possible and emulating support where not. When looking at the bit shift decomposition I saw what seems to be a bug in the implementation. The affected function is ExpandShiftWithUnknownAmountBit in LegalizeIntegerTypes.cpp. Below is the original code and the proposed fix. Could someone please review the changes? If they are correct how do I go about submitting a patch? Thanks, Javier [Original] /// ExpandShiftWithUnknownAmountBit - Fully general expansion of integer shift /// of any size...

Hi Javier, > The problem is the implementation of the expansion. Perhaps an example > can help illustrate better. Take the case of a 64-bit integer shifted > left by say 6 bits and is decomposed using 32-bit registers. Because 6 > is less than the 32 (the register size) the resulting low part should be > equal to the source low part shifted left by 6 bits. The current >

...egers to my target. I'm using > LLVM to decompose the 64-bit integer operations by using 32-bit registers > wherever possible and emulating support where not. When looking at the bit > shift decomposition I saw what seems to be a bug in the implementation. The > affected function is ExpandShiftWithUnknownAmountBit in > LegalizeIntegerTypes.cpp. Below is the original code and the proposed fix. > Could someone please review the changes? If they are correct how do I go > about submitting a patch? [snip] Please use "svn diff" to generate proposed patches; the form you used is extremely diffi...

...M to decompose the 64-bit integer operations by using 32-bit >> registers >> wherever possible and emulating support where not. When looking at >> the bit >> shift decomposition I saw what seems to be a bug in the >> implementation. The >> affected function is ExpandShiftWithUnknownAmountBit in >> LegalizeIntegerTypes.cpp. Below is the original code and the proposed >> fix. >> Could someone please review the changes? If they are correct how do I go >> about submitting a patch? > > can you please describe the problem and how you fix it (in words, not >...

...de you pasted came from the function >> ExpandShiftByConstant and indeed it looks correct. In my example I used 6 >> as the shift amount but forgot to mention that it's stored in a register. > > I see, sorry I didn't read your email more carefully. It does look like > ExpandShiftWithUnknownAmountBit is bogus - at a glance it looks like the > Cmp condition is inverted. I may have time to take a closer look tomorrow. > > Best wishes, > > Duncan.

Hi Javier, > Just wondering if you had a chance to look at the expansion. I wrote my own fix and committed it in revision 90482. Do you have a testcase I can add to the testsuite (preferably not an execution test, but rather something that checks the assembler being generated)? Ciao, Duncan.

PS: For a small optimization, in the case where Amt is bigger than 32 (or whatever NVTBits is) you might want to use an "and" to mask off the top bits of Amt rather than subtracting 32 (if Amt is 64 or greater then the result of the shift was undefined anyway, so it is ok to mask off all the upper bits).

Hi Javier, > The issue with a test case is that it will depend on the target to expose > it. Like you noted in the check-in comment x86 doesn't expose the bug. The > target I'm working on isn't public. Short of writing a new one for the > purpose of a test case I don't know what else to do. Do you have a > suggestion? in that case I guess we will have to live

Hi Javier, > I don't know if the optimization would help us much. Our architecture > performs integer shifts and subtractions at the same speed. I think the > optimization is limited to the case where NVBits is a power of two. it is irrelevant whether it is a power of two or not. Anyway, since you are the only user, and this is not helpful for you, I guess we can forget it :) >

Hi Duncan, Maybe I'm beating a dead horse but I wanted to see if understood correctly your optimization to avoid doing a subtraction. The way I understand it the idea is to mask out the high bits of Amt which in some cases is equivalent to subtracting a value from Amt. The cases where this holds is when the value subtracted from Amt is a power of two. This value is half the original register

Hello, A while back I submitted a patch for this function. The patch, added in revision 90482, left the case of a shift by zero undefined. Attached is another patch to deal with that condition. Thanks, Javier -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100824/a66bd3d8/attachment.html> --------------

Hi Duncan, I don't know if the optimization would help us much. Our architecture performs integer shifts and subtractions at the same speed. I think the optimization is limited to the case where NVBits is a power of two. That is the case for all current value types but there's a separate thread where someone is proposing adding i20 as a native type. In your previous email you mentioned

Duncan, Thanks for committing a fix. I see that you fixed a bug in my patch with the HiL in the case of an SRA. The issue with a test case is that it will depend on the target to expose it. Like you noted in the check-in comment x86 doesn't expose the bug. The target I'm working on isn't public. Short of writing a new one for the purpose of a test case I don't know what else to

Hi All, I am looking at a linker error under O2: undefined symbol __lshrdi3 I have two questions here: 1. Does that mean our libgcc (?) doesn't implement __lshrdi3? Or more generally, why I have such linker error? 2. Seems some operations are combined, and replaced with __lshrdi3 call. I am interested in when such libcall will be generated? Could you show me one