src/mongo/db/fts/unicode is not optimised on ppc64el

Bug #1758118 reported by Robie Basak
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
mongodb (Ubuntu)
Fix Released
Medium
Unassigned

Bug Description

I'm about to regress performance in mongodb on ppc64el by using the non-optimised path to fix bug 1758116, so am opening this bug to track the consequent performance regression.

When https://jira.mongodb.org/browse/SERVER-33395 is fixed, we should be able to re-enable this path.

Revision history for this message
Robie Basak (racb) wrote :

I'm wary of the suggested change in the upstream bug report:

- return vec_extract(vec_vbpermq(_data, bits), 0);
+ return vec_extract(vec_vbpermq(_data, bits), 1);

It's useful to know if this fixes the problem, and I can confirm that it does. But without knowing _why_ this fixes the issue, it isn't clear to me that this is a correct fix, and so I did not include it. Since otherwise that'd suggest a significant behaviour change from older gcc, and I haven't found a relevant compiler bug report that indicates that this is the case.

I'd appreciate someone who understands the ISA and compiler behaviour to verify what the correct fix should be, advocate that upstream and get it landed. Then we can cherry-pick that fix easily enough and re-enable the ppc64el optimisation path.

Frank Heimes (fheimes)
tags: added: ppc64el
bugproxy (bugproxy)
tags: added: architecture-ppc64le bugnameltc-166135 severity-medium targetmilestone-inin---
Revision history for this message
William J. Schmidt (wschmidt-g) wrote :

There was a bug in GCC that could cause incorrect code to be generated for -mcpu=power8 in a computation involving vec_vbpermq. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84033.

This bug has been fixed and backported to supported releases (GCC 6 and 7). Please ensure that your toolchain team picks up the fix before making any source code changes. I apologize for the problem you've run into here.

I don't have ability to comment on the upstream bug report; could someone please pass this information along?

Revision history for this message
Breno Leitão (breno-leitao) wrote :

I wrote a very small code that mimics this problem, it is at:

  https://github.com/leitao/altivec

I found that the information we are looking for (i.e, the bitmap) is at the second half of the vector, i.e, vec_extract(VSR, 1) will return the correct value of the output of the perm value, and vec_extract(VSR, 0) will always return zero, independent of the Vector input and the bit mask.

That said, it seems that the change makes sense on current compilers, but, I am wondering why it changed from GCC 5.4 to current GCC.

I will test the same code on old compilers and see if I get a different behavior. I suspect that there was a change on GCC (Maybe the one provided by 84033?) that caused this element order change.

Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- Comment From <email address hidden> 2018-03-28 15:41 EDT-------
Yes, it could be that they picked up the fix and exposed the problem in the user code, which worked with the broken compiler. That's consistent with the nature of the bug.

If this code is used on both big- and little-endian systems, the index of the vec_extract changes. It should be 0 for big-endian and 1 for little-endian. The vbpermq instruction isn't endian-sensitive, and always places it in the high-order half of the register.

Revision history for this message
William J. Schmidt (wschmidt-g) wrote :

Going back to c#1, though, I can definitely say that using 1 rather than 0 is correct behavior on little-endian machines. The question is whether this code is being used for big-endian also. If so, then this change will break the code on big-endian machines. This code needs to be endian-aware.

Revision history for this message
Gianfranco Costamagna (costamagnagianfranco) wrote :

Lets close this now that we adopted the Debian patch version 8and upstream one)

Changed in mongodb (Ubuntu):
status: Triaged → Fix Released
bugproxy (bugproxy)
tags: added: targetmilestone-inin18043
removed: targetmilestone-inin---
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.